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

Ks 2022 09 proposals random sorting #4475

Merged
merged 3 commits into from
Sep 16, 2022

Conversation

Rineee
Copy link
Contributor

@Rineee Rineee commented Sep 14, 2022

The random stuff is copied from here. Im a bit unhappy with doubling everything, but did not know how else to do it in a good way?

ToDo:

  • add and fix tests

Copy link
Contributor

@khamui khamui left a comment

Choose a reason for hiding this comment

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

working fine!

the preserved variable is hard to read for me, but its my inexperience :) Leaving for @fuzzylogic2000 to doublecheck!

if ordering:
if ordering == ['-daily_random']:
pks = list(queryset.values_list('pk', flat=True))
random.seed(str(date.today()))
Copy link
Contributor

Choose a reason for hiding this comment

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

nice!

@Rineee Rineee changed the title Ks 2022 09 proposals random sorting WIP Ks 2022 09 proposals random sorting Sep 15, 2022
@Rineee Rineee force-pushed the ks-2022-09-proposals-random-sorting branch from b477814 to 6d078b8 Compare September 15, 2022 16:04
@Rineee Rineee changed the title WIP Ks 2022 09 proposals random sorting Ks 2022 09 proposals random sorting Sep 15, 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! Also, more testing. :)

@fuzzylogic2000 fuzzylogic2000 force-pushed the ks-2022-09-proposals-random-sorting branch from 6d078b8 to b13d64c Compare September 16, 2022 06:32
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.

It looks good, but it seems like the newest filter breaks with that? (Fine on dev, but different to the second list locally.) Will investigate more later!

@@ -85,12 +85,13 @@ def list(self, request, *args, **kwargs):
ordering_choices = [('-created', _('Most recent')), ]
if self.module.has_feature('rate', Proposal):
ordering_choices += ('-positive_rating_count', _('Most popular')),
ordering_choices += ('-comment_count', _('Most commented')),
ordering_choices += ('-comment_count', _('Most commented')), \
('-daily_random', _('Random')),
Copy link
Contributor

Choose a reason for hiding this comment

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

Why the -? It probably doesn't matter if we use the random ascending or descending.

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.

It is all fine looking at it again. And also it doesn't make sense that the code shouldn't work like before.

@fuzzylogic2000 fuzzylogic2000 merged commit 82e6d1e into main Sep 16, 2022
@fuzzylogic2000 fuzzylogic2000 deleted the ks-2022-09-proposals-random-sorting branch September 16, 2022 09:33
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