Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve createFromStructuredData() #237

Merged
merged 3 commits into from
Nov 25, 2023
Merged

Conversation

kylekatarnls
Copy link
Collaborator

  • Allow empty array for createFromStructuredData() (as specs allow it)
  • Make OpeningHoursSpecificationParser constructor private and add separated named constructors
  • Add details in exceptions to know what is the issue precisely and which item is incorrect
  • Accept empty array as per specs for structured-data
  • Add utils to know if opening-hours are always open/closed
  • Add OpeningHours::every() method
  • Make 23:59 short for 24:00 in createFromStructuredData (the way to represent 24h/24 for OpeningHours)
  • Make range where closes < opens span over night (as per specs)

- Make OpeningHoursSpecificationParser constructor private and add separated named constructors
…hich item is incorrect

- Accept empty array as per specs for structured-data
- Add utils to know if opening-hours are always open/closed
- Add OpeningHours::every() method
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (78578d8) 98.05% compared to head (20eb028) 100.00%.

❗ Current head 20eb028 differs from pull request most recent head 793b060. Consider uploading reports for the commit 793b060 to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@              Coverage Diff              @@
##             master      #237      +/-   ##
=============================================
+ Coverage     98.05%   100.00%   +1.94%     
- Complexity      387       400      +13     
=============================================
  Files            26        26              
  Lines           924       962      +38     
=============================================
+ Hits            906       962      +56     
+ Misses           18         0      -18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kylekatarnls
Copy link
Collaborator Author

kylekatarnls commented Nov 25, 2023

Hello @ambroisemaupate I made some changes to get a bit closer to specs such as allowing ranges over night by default. Implement 23:59 shortcut for end-of-day (not the same meaning as in OpeningHours class) and try to deal with invalid data with more verbose errors.

I also removed the check throwing exception when data is empty as it may be a valid case for some businesses and is allowed by the specs.

However I added a ->isAlwaysClosed() util so one can test after creation and throw an exception if it's not a valid case for the business.

@kylekatarnls kylekatarnls merged commit dd78539 into master Nov 25, 2023
14 checks passed
@kylekatarnls kylekatarnls deleted the feature/structured-data branch November 25, 2023 16:38
@ambroisemaupate
Copy link
Contributor

Awesome @kylekatarnls , thanks for your review and improvements.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants