-
Notifications
You must be signed in to change notification settings - Fork 31
Add a filter to sequences to filter on universe #690
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
base: beta
Are you sure you want to change the base?
Add a filter to sequences to filter on universe #690
Conversation
jochengcd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is filtering on the story universe, not the character universe ? To clarify, we maybe should label this with 'Story Universe' and not 'Universe' ?
Another thing, in difference to the other filters, here 'None' is a valid selection, i.e. filter for those stories without a universe set. Can we look into adding that ? Not sure if feasible, but the question will come from users.
801ec4c to
04c856f
Compare
|
This is filtering on the story universe, not the character universe ? To clarify, we maybe should label this with 'Story Universe' and not 'Universe' ? Correct, have updated for that before I forget. Another thing, in difference to the other filters, here 'None' is a valid selection, i.e. filter for those stories without a universe set. Can we look into adding that ? Not sure if feasible, but the question will come from users. I will give it a try :) |
| if publishers: | ||
| qs = Publisher.objects.filter(id__in=publishers) | ||
| self.form['publisher'].field.queryset = qs | ||
| if universe: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking further, I don't think that this should in CommonFilter, since it only applies to Sequences ?
| universes = set(self.qs.values_list(universe, flat=True)) | ||
| universes.discard(None) # Remove None before checking if set is empty | ||
| if universes: | ||
| qs = Universe.objects.filter(id__in=universes).select_related('verse') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style guide of code is less then 80 characters
| self.form['publisher'].field.queryset = qs | ||
| if universe: | ||
| universes = set(self.qs.values_list(universe, flat=True)) | ||
| universes.discard(None) # Remove None before checking if set is empty |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
adding
null_label='No Universe',
to universe = ModelMultipleChoiceFilter(...) will do the filtering on no universe set, which we should support.
But then the universe filter will not show when selecting that, so unsetting only by going back, which is not ideal. Removing will show the universe box for characters without any, which we don't want either.
Can there be a different check ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if 'null' not in self.data.getlist('universe'):
universes.discard(None) # Remove None before checking if set is empty
|
Another aspect to consider, universe will show on all sequence list, also for creators, features, ... Which I am not sure we want ? |
|
I also wonder now if we want this at all, we have pages such as https://www.comics.org/character/6122/issues/character_universe/3/ and your story type filter solved my immediate need. We also have https://www.comics.org/universe/3/sequences/ The more I think about it, the more i wonder if this is adds the cumbersome of a filter that the users won't really need. |
|
Maybe ask on main on use cases ? We can:
We cannot:
But can we make use of that ? Adding many filters (at least in the current way) can become unwidely. |
|
Yeah i'll ask the question - and we can find issues of a character set in a specific universe with https://www.comics.org/character/6261/issues/story_universe/2/ - so just boils down to if its worth it for 1) sequences only and 2) selecting multiple story universes. |
Tested with all sequence views, some examples:
Character sequences unfiltered

with filter:

Feature:

Creator:

Universe correctly pops it:

For anything not involved in Marvel/DC, universes filter doesnt appear whenno sequences have a universe in data to filter:

And then finally, the addition of story type filter I noticed meant we started to overhang on mobile, this would have made it worse, so added a wrap, example on mobile size:
