# |
Mar 10th 2018, 16:50 |
bravo-kernel |
If I am missing something please let me know. Updating the ticket from here on, sorry for the noise |
# |
Mar 10th 2018, 16:49 |
bravo-kernel |
Unsure how this should be done technically but I can imagine the the query parameters must somehow be retained |
# |
Mar 10th 2018, 16:48 |
bravo-kernel |
The links inside meta would currently (probably) not point to the correct next page |
# |
Mar 10th 2018, 16:48 |
bravo-kernel |
I can imagine getting two pages of results using your "sort" |
# |
Mar 10th 2018, 16:47 |
bravo-kernel |
2. the JSON:API pagination results |
# |
Mar 10th 2018, 16:46 |
bravo-kernel |
1. the `direction` query param (which is most likely already functioning) |
# |
Mar 10th 2018, 16:45 |
bravo-kernel |
Mr. @rchavik I can come up with two things reading the ticket carefully |
# |
Mar 10th 2018, 16:06 |
bravo-kernel |
I am unsure myself too to be honest, I will check as well |
# |
Mar 10th 2018, 16:00 |
rchavik |
sparse field support :slightly_smiling_face: |
# |
Mar 10th 2018, 15:59 |
rchavik |
oh, https://github.com/FriendsOfCake/crud-json-api/pull/31 |
# |
Mar 10th 2018, 15:59 |
rchavik |
there's already CrudJsonApi.Pagination listener |
# |
Mar 10th 2018, 15:55 |
rchavik |
i thought pagination is already working |
# |
Mar 10th 2018, 15:55 |
rchavik |
not yet, that PR only implements the 'sort' support |
# |
Mar 10th 2018, 15:52 |
rchavik |
not sure.. i'll check ticket again |
# |
Mar 10th 2018, 15:52 |
bravo-kernel |
I read a note in the ticket about pagination. Is that accounted for too? |
# |
Mar 10th 2018, 15:51 |
bravo-kernel |
@rchavik that’s quite impressive. I will take a look in about an hour. |
# |
Mar 10th 2018, 15:38 |
admad |
Nifty https://github.com/san-kumar/lambdaphp |
# |
Mar 10th 2018, 14:14 |
rchavik |
https://github.com/FriendsOfCake/crud-json-api/pull/29 is basically done.. found a event handler leak in tests as well |
# |
Mar 10th 2018, 12:44 |
bravo-kernel |
as you were, thanks |
# |
Mar 10th 2018, 12:44 |
bravo-kernel |
right, I will ask that as well |
# |
Mar 10th 2018, 12:44 |
admad |
the change looks good, though would be nice to have an actual test with custom foreign key |
# |
Mar 10th 2018, 12:44 |
bravo-kernel |
Much much obliged, thanks |
# |
Mar 10th 2018, 12:43 |
bravo-kernel |
thanks |
# |
Mar 10th 2018, 12:43 |
bravo-kernel |
:thumbsup: |
# |
Mar 10th 2018, 12:43 |
admad |
yes |
# |
Mar 10th 2018, 12:43 |
bravo-kernel |
so `?` as the key |
# |
Mar 10th 2018, 12:43 |
bravo-kernel |
aha |
# |
Mar 10th 2018, 12:43 |
admad |
that's how query string should always be set :slightly_smiling_face: to avoid any potential clash with route params |
# |
Mar 10th 2018, 12:43 |
bravo-kernel |
haha |
# |
Mar 10th 2018, 12:43 |
bravo-kernel |
whoa, what's that? |
# |
Mar 10th 2018, 12:42 |
admad |
then better to use `'?' => [$association->foreignKey() => $entity->id]` |
# |
Mar 10th 2018, 12:42 |
bravo-kernel |
yes, like this example `"self": "/cultures?country_id=2"` |
# |
Mar 10th 2018, 12:37 |
admad |
is the 'fk' => $entity->id supposed to endup in URLs query string? |
# |
Mar 10th 2018, 12:30 |
bravo-kernel |
@admad would you mind taking a quick look at this one-line PR? https://github.com/FriendsOfCake/crud-json-api/pull/26/files. Tests pass and looks ok but I would prefer confirmation before merging |
# |
Mar 10th 2018, 12:17 |
admad |
thanks |
# |
Mar 10th 2018, 12:07 |
steinkel |
sure, I'll do that |
# |
Mar 10th 2018, 12:06 |
admad |
and please submit to awesome list if it's not already there :slightly_smiling_face: |
# |
Mar 10th 2018, 12:06 |
gsitex |
@admad :+1: |
# |
Mar 10th 2018, 12:06 |
admad |
@steinkel perhaps the plugin should be suggested in error message here https://github.com/cakephp/app/blob/master/config/requirements.php#L31 |
# |
Mar 10th 2018, 12:05 |
gsitex |
I am not a fan of modifying files in `vendor` folder. But it's a hack anyway. |
# |
Mar 10th 2018, 12:05 |
gsitex |
@steinkel `use IntlCalendar as IC;` fixed the issue ;) |