Skip to content

Conversation

@carlobeltrame
Copy link
Member

@carlobeltrame carlobeltrame commented May 14, 2025

Based on #7431 so the e2e tests can pass, only review the second commit here.

When fetching periods via the /periods endpoint, they were ordered by createTime DESC. This is different from fetching the periods of a specific camp, where they were ordered by start ASC. As a result, the firstFuturePeriod, i.e. the period displayed by default when navigating to the picasso, may be different depending on whether the last reload was on the camp list or the dashboard.

Simply adding the order: entry to the ApiResource annotation was not enough: It made the SQL order by createTime DESC, start ASC, which is unhelpful.
So I had to remove the default ordering. Not sure if this is a bug or a feature of API Platform.

As a side effect, now the content types are correctly ordered by name too.

@carlobeltrame carlobeltrame force-pushed the fix-entity-ordering branch from b683b5d to f94a559 Compare May 15, 2025 08:24
Otherwise some of the camps would become past camps, making the e2e
tests fail because they do not show up any more by default on the camp
list
@carlobeltrame carlobeltrame force-pushed the fix-entity-ordering branch 2 times, most recently from ca33a3c to 4700808 Compare May 15, 2025 08:51
The default ordering by createTime DESC had to be removed for this to
take effect. As a side effect, content types are also ordered correctly
according to name ASC now, as we specify on the entity class.
@carlobeltrame carlobeltrame force-pushed the fix-entity-ordering branch from 4700808 to 9c642c1 Compare May 15, 2025 09:06
@carlobeltrame carlobeltrame changed the title Fix period ordering Consistently open the next future period in the picasso May 15, 2025
@pmattmann pmattmann requested a review from a team May 16, 2025 21:33
Copy link
Member

@manuelmeister manuelmeister left a comment

Choose a reason for hiding this comment

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

Thanks!

@carlobeltrame carlobeltrame added this pull request to the merge queue May 18, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 18, 2025
@carlobeltrame carlobeltrame added this pull request to the merge queue May 18, 2025
Merged via the queue into ecamp:devel with commit 10509bc May 18, 2025
30 checks passed
@carlobeltrame carlobeltrame deleted the fix-entity-ordering branch May 18, 2025 19:13
@carlobeltrame carlobeltrame mentioned this pull request Jun 10, 2025
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.

3 participants