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

DDFHER-101- Allow retrieval of opening hours for multiple branches #1659

Conversation

@kasperbirch1 kasperbirch1 force-pushed the DDFHER-101-retrieval-of-opening-hours-for-multiple-libraries-in-api branch 2 times, most recently from c2ca064 to 76dec5c Compare October 21, 2024 12:51
Copy link
Contributor

@kasperg kasperg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciate the effort but I think this needs another round on multiple levels.

@kasperg kasperg assigned kasperbirch1 and unassigned kasperg and xendk Oct 22, 2024
@kasperbirch1 kasperbirch1 force-pushed the DDFHER-101-retrieval-of-opening-hours-for-multiple-libraries-in-api branch from 76dec5c to 4a70427 Compare October 23, 2024 11:47
Updated the `OpeningHoursRepository::loadMultiple` method to accept  an array of branch IDs, enabling queries for multiple branches using the `IN` operator.

In OpeningHoursResource, the `branch_id` / `nid` parameter now accepts a single ID or a comma-separated list of IDs.

These changes maintain backward compatibility while enhancing the `redia_legacy` API to support retrieval of opening hours for multiple branches in a single request.
@kasperbirch1 kasperbirch1 force-pushed the DDFHER-101-retrieval-of-opening-hours-for-multiple-libraries-in-api branch from 4a70427 to 066ae5f Compare October 23, 2024 11:54
@kasperbirch1 kasperbirch1 requested a review from kasperg October 23, 2024 12:21
@kasperbirch1 kasperbirch1 assigned kasperg and unassigned kasperbirch1 and kasperg Oct 23, 2024
@kasperbirch1 kasperbirch1 reopened this Oct 23, 2024
@kasperg kasperg assigned kasperbirch1 and unassigned kasperg Oct 24, 2024
Since the specification permits only a single integer for branch_id, we should continue using getInt. We’ll wrap it in an array with getInt('branch_id') as the sole item.
`PREG_NO_ERROR` is not a valid flag for `preg_split()`, and the intended behavior is to exclude empty strings from the resulting array.
Return an empty array if getInt('branch_id') is null.
````
Parameter #1 $branchIds of method
Drupal\dpl_opening_hours\Model\OpeningHoursRepository::loadMultiple()
expects array<int>, array<int, int|null> given.
```
@kasperbirch1 kasperbirch1 requested a review from kasperg October 24, 2024 11:11
@kasperbirch1 kasperbirch1 assigned kasperg and unassigned kasperbirch1 Oct 24, 2024
Copy link
Contributor

@kasperg kasperg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 LGTM

@kasperg kasperg assigned kasperbirch1 and unassigned kasperg Oct 28, 2024
@kasperbirch1 kasperbirch1 merged commit 09d4630 into develop Oct 29, 2024
23 checks passed
@kasperbirch1 kasperbirch1 deleted the DDFHER-101-retrieval-of-opening-hours-for-multiple-libraries-in-api branch October 29, 2024 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants