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

fix: Querying all trips from all accounts #391

Merged
merged 1 commit into from
Feb 8, 2024
Merged

fix: Querying all trips from all accounts #391

merged 1 commit into from
Feb 8, 2024

Conversation

Merkur39
Copy link
Member

@Merkur39 Merkur39 commented Feb 8, 2024

This query should not sort the accounts, because since there is a limitBy, we can run into a case where we don't see all the latest trips.
The presence in the .select is not necessary either.

Copy link

bundlemon bot commented Feb 8, 2024

BundleMon

Unchanged files (4)
Status Path Size Limits
vendors/coachco2.(hash).js
1.65MB -
app/coachco2.(hash).js
67.64KB -
vendors-coachco2.(hash).(hash).min.css
35.96KB -
app-coachco2.(hash).min.css
356B -

Total files change +5B 0%

Final result: ✅

View report in BundleMon website ➡️


Current branch size history | Target branch size history

{ endDate: 'desc' }
])
.select(['startDate', 'endDate', 'title', 'aggregation'])
.indexFields(['startDate', 'endDate'])
Copy link
Contributor

@paultranvan paultranvan Feb 8, 2024

Choose a reason for hiding this comment

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

The endDate does not seem necessary ? I guess the sort on startDate is enough

Copy link
Member Author

@Merkur39 Merkur39 Feb 8, 2024

Choose a reason for hiding this comment

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

It's necessary to have the endDate in the app(for duration trip calculations at least), but indeed it is not useful in the .sortBy

Copy link
Contributor

Choose a reason for hiding this comment

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

And if it's not useful in the sortBy, it's not useful in the indexField either ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

Seen together, pending the correction of this issue cozy/cozy-client#1216, endDate cannot be removed from the indexFields

@Merkur39 Merkur39 force-pushed the fix/ver-119 branch 2 times, most recently from fb98b25 to d90cb14 Compare February 8, 2024 10:37
This query should not sort the accounts,
because since there is a limitBy we can run into a case
where we don't see all the latest trips.
The presence in the .select is not necessary either.
@Merkur39 Merkur39 merged commit 4a68140 into master Feb 8, 2024
1 check passed
@Merkur39 Merkur39 deleted the fix/ver-119 branch February 8, 2024 11:27
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