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 order in annotation comments with same score #3565

Merged
merged 2 commits into from
May 30, 2019

Conversation

javierm
Copy link
Member

@javierm javierm commented May 29, 2019

References

Objectives

  • Fix inconsistent order for comments in annotations, which caused some tests to fail.

Notes

The specs failed because comments in annotations are ordered by score. However, when several comments have the same score, no other order is defined. This meant specs assuming the last comment displayed was the last comment created failed sometimes.

Using ActiveRecord relations instead of arrays makes it easy to reuse scopes used everywhere else in the code, and so comments with the same score are now ordered by date.

By setting the comments lazily in a different method, we can share its
code with the parent class.
Using arrays made it difficult to order by more than one field (like the
`most_voted` scope does), and so we were ordering by `confidence_score`
and ignoring the `created_at` column.

Using an AR relation makes it easy to reuse the existing `most_voted`
scope.

This change has one side effect: now comments with equal votes are
ordered in descending order instead of having no specific order. That
means flaky specs which failed sometimes because they assumed comments
were ordered by date are now always green.

I've also re-added the `oldest` scope removed in 792b15b thinking it was
removed because using it with arrays was too hard.
@javierm javierm self-assigned this May 29, 2019
@javierm javierm merged commit 087a21b into master May 30, 2019
@javierm javierm deleted the fix_annotation_comments_order branch May 30, 2019 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hunt the flaky - Commenting legislation questions Turns links into html links
2 participants