-
Notifications
You must be signed in to change notification settings - Fork 1
feature: add Hours endpoint #1
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Lits\LibCal\Action; | ||
|
|
||
| use Lits\LibCal\Action; | ||
| use Lits\LibCal\Client; | ||
| use Lits\LibCal\Data\Hours\HoursData; | ||
| use Lits\LibCal\Exception\ActionException; | ||
|
|
||
| /** Action to get opening hours for a location. */ | ||
| class HoursAction extends Action | ||
| { | ||
| use TraitIdMultiple; | ||
| use TraitCache; | ||
| use TraitCache; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Duplicate |
||
|
|
||
| public ?string $from = null; | ||
| public ?string $to = null; | ||
|
|
||
| /** | ||
| * Set the date to retrieve hours from. | ||
| * | ||
| * @param string|\DateTimeInterface|null $from Value to set. Strings should | ||
| * be in yyyy-mm-dd format | ||
| * @return self A reference to this object for method chaining. | ||
| * @throws ActionException If an invalid date is specified. | ||
| */ | ||
| final public function setFrom($from): self | ||
| { | ||
| $this->from = self::buildDateString($from); | ||
|
|
||
| return $this; | ||
| } | ||
|
|
||
| /** | ||
| * Set the date to retrieve hours to. | ||
| * | ||
| * @param string|\DateTimeInterface|null $to Value to set. Strings should | ||
| * be in yyyy-mm-dd format | ||
| * @return self A reference to this object for method chaining. | ||
| * @throws ActionException If an invalid date is specified. | ||
| */ | ||
| final public function setTo($to): self | ||
| { | ||
| $this->to = self::buildDateString($to); | ||
|
|
||
| return $this; | ||
| } | ||
|
|
||
| /** | ||
| * @return HoursData[] | ||
| * @throws \Lits\LibCal\Exception\ClientException | ||
| */ | ||
| final public function send(): array | ||
| { | ||
| $uri = '/' . Client::VERSION . '/hours'; | ||
| $uri = $this->addId($uri); | ||
| $uri = self::addQuery($uri, 'from', $this->from); | ||
| $uri = self::addQuery($uri, 'to', $this->to); | ||
|
|
||
| /** @var HoursData[] $result */ | ||
| $result = $this->memoize( | ||
| $uri, | ||
| fn (string $uri) => HoursData::fromJsonAsArray( | ||
| $this->client->get($uri) | ||
| ) | ||
| ); | ||
|
|
||
| return $result; | ||
| } | ||
|
|
||
| /** | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the buildDateString() and dateStringIsValid() static methods should just be put in the abstract Action class, so then they could also be used by TraitDate instead of duplicating code. |
||
| * Return a date string in 'yyyy-mm-dd' format. | ||
| * | ||
| * @param string|\DateTimeInterface|null $date The date. | ||
| * @return string|null The date in 'yyyy-mm-dd' format. Null if null input. | ||
| * @throws ActionException If an invalid date is specified. | ||
| */ | ||
| private static function buildDateString($date): ?string | ||
| { | ||
| if (\is_null($date)) { | ||
| return null; | ||
| } | ||
|
|
||
| if ($date instanceof \DateTimeInterface) { | ||
| return $date->format('Y-m-d'); | ||
| } | ||
|
|
||
| if (!self::dateStringIsValid($date)) { | ||
| throw new ActionException('Invalid date specified'); | ||
| } | ||
|
|
||
| return $date; | ||
| } | ||
|
|
||
| /** | ||
| * Is string date in 'yyyy-mm-dd' format? | ||
| * | ||
| * @param string $date Value to set. | ||
| * @return bool true if the string is in 'yyyy-mm-dd' format, | ||
| * false if not | ||
| */ | ||
| private static function dateStringIsValid(string $date): bool | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd probably name this isValidDateString() instead. |
||
| { | ||
| $date = \filter_var( | ||
| $date, | ||
| \FILTER_VALIDATE_REGEXP, | ||
| ['options' => ['regexp' => '/^\d{4}-\d{2}-\d{2}$/']] | ||
| ); | ||
|
|
||
| return $date !== false; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
|
|
||
| namespace Lits\LibCal; | ||
|
|
||
| use Lits\LibCal\Action\HoursAction; | ||
| use Lits\LibCal\Action\SpaceAction; | ||
| use Lits\LibCal\Data\TokenData; | ||
| use Lits\LibCal\Exception\ClientException; | ||
|
|
@@ -182,6 +183,17 @@ public function space(): SpaceAction | |
| return new SpaceAction($this); | ||
| } | ||
|
|
||
| /** | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Organizationally, this should be before the space() method in this class. |
||
| * Retrieve action for LibCal Hours API. | ||
| * | ||
| * @param int|int[] $id The id of the location to retrieve. | ||
| * @return HoursAction Hours lookup action. | ||
| */ | ||
| public function hours($id): HoursAction | ||
| { | ||
| return new HoursAction($this, $id); | ||
| } | ||
|
|
||
| /** | ||
| * Get any memoized value from cache or memory. | ||
| * | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Lits\LibCal\Data\Hours; | ||
|
|
||
| use Lits\LibCal\Data; | ||
|
|
||
| class DateData extends Data | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Class should be final. |
||
| { | ||
| public ?string $status = null; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd probably have $status be non-nullable as I'm guessing it is always provided. |
||
|
|
||
| public ?string $note = null; | ||
|
|
||
| /** @var OpeningHoursData[] */ | ||
| public array $hours = []; | ||
| } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To match the /hours endpoint being at the root, I'd move this file up one directory/namespace so it is Lits\LibCal\Data\HoursData. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Lits\LibCal\Data\Hours; | ||
|
|
||
| use Lits\LibCal\Data; | ||
|
|
||
| class HoursData extends Data | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Class should be final. |
||
| { | ||
| public ?int $lid = null; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I generally left properties in these classes non-nullable when the API returned a value, and it looks like that happens for everything but $lon and $fn for some reason. I probably could be convinced to allow them to be nullable though. |
||
| public ?string $name = null; | ||
| public ?string $category = null; | ||
| public ?string $desc = null; | ||
|
|
||
| public ?string $url = null; | ||
| public ?string $contact = null; | ||
| public ?string $lat = null; | ||
| public ?string $lon = null; | ||
| public ?string $color = null; | ||
| public ?string $fn = null; | ||
|
|
||
| public string $parent_lid; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be nullable, as it isn't provided in the response for a whole Library. |
||
|
|
||
| /** @var DateData[] $dates */ | ||
| public array $dates = []; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The type of $dates in the comment should be: |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| namespace Lits\LibCal\Data\Hours; | ||
|
|
||
| use Lits\LibCal\Data; | ||
|
|
||
| class OpeningHoursData extends Data | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Class should be final. |
||
| { | ||
| public ?string $from = null; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd probably make both $from and $to non-nullable as well. |
||
| public ?string $to = null; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Entire class should be final instead of individual methods.