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

budgeting//contrib/filters: add random ordering to ProposalFilterSet … #4582

Merged
merged 2 commits into from
Oct 20, 2022

Conversation

Rineee
Copy link
Contributor

@Rineee Rineee commented Oct 19, 2022

…and rename filter in api to be able to pass it

depends on liqd/adhocracy4#1246

fixes #4574

I had to add the daily random sorting to the list view as otherwise the annotations dont work and thus dont show up in the map pop ups. To make it work with the api filter, I had to change the wording there to be consistent with the wording in the list filte in a4.

The daily random sorting is different for the django list and react list, that is because in the react list we are only ordering the proposals of a certain module whereas in the django list, the ordering is determined on all proposal in the db and then later the proposals are filtered for the module. I did not dare to touch that as that is deep in a4 and django. But since we will always have either or, it should be fine?

@Rineee Rineee added the Dev: A4 depending PR or issue dependent on A4 label Oct 19, 2022
Copy link
Contributor

@fuzzylogic2000 fuzzylogic2000 left a comment

Choose a reason for hiding this comment

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

Nice!
And an interesting find, that we order the Django a4-one before filtering for the module. But I also think you are right, it doesn't matter as we always use either or and don't need the same random ordering in both cases.
Happy that you merge when you updated a4!

@@ -112,12 +112,12 @@ def list(self, request, *args, **kwargs):
if has_feature_active(self.module, Proposal, 'support'):
ordering_choices += ('-positive_rating_count', _('Most support')),
ordering_choices += ('-comment_count', _('Most commented')), \
('-daily_random', _('Random')),
('dailyrandom', _('Random')),
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 It doesn't matter if it's ascending or descending random. 🤣

@Rineee Rineee force-pushed the ks-2022-10-fix-annotations-dailyrandom branch from 1de0b9a to 864e8e7 Compare October 20, 2022 15:13
@Rineee Rineee merged commit 418275a into main Oct 20, 2022
@Rineee Rineee deleted the ks-2022-10-fix-annotations-dailyrandom branch October 20, 2022 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Dev: A4 depending PR or issue dependent on A4
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Budgeting: Votes are not shown on Map Pins
2 participants