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

Fixed category labels not being populated #3778

Merged
merged 3 commits into from
Feb 22, 2024
Merged

Conversation

wes-otf
Copy link
Contributor

@wes-otf wes-otf commented Feb 19, 2024

Description

Fixes #3741. Adds a new tag to get the label of the category from the ID

Test Steps

  • Navigate to the beta view
  • Select a new category to filter on
  • When a chip pops up to indicate an active category filter, ensure it is the same as the label that was previously selected, not a number.

@wes-otf wes-otf requested review from frjo and theskumar February 19, 2024 22:14
@wes-otf wes-otf added Type: Bug Bugs! Things that are broken :-/ Type: Patch Mini change, used in release drafter labels Feb 19, 2024
@frjo
Copy link
Member

frjo commented Feb 20, 2024

I think this PR look good but the feature over all is not useful it its current state. See #3269

I do not think category filtering active on OTF live?

@wes-otf
Copy link
Contributor Author

wes-otf commented Feb 20, 2024

@frjo Ah that issue is new to me. It works on live with categories that have lower numbers of submissions but I am seeing that SystemExit with other categories.

@theskumar
Copy link
Member

Looking at the Traceback, the SystemExit seems to be a performance issue trying to filter by category, we can have another ticket to resolve that, the filter by category which uses django-filter should be optimized so it doesn't loop-over all the submissions, trying to find and filter by category individually. I can take a stab at it.

Screenshot 2024-02-21 at 6  15 00@2x

Returns:
A representation of the label
"""
return str(Option.objects.get(id=label_id))
Copy link
Member

Choose a reason for hiding this comment

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

This looks good, but would generally suggest dealing with it in view.py code itself, that way it's easier to fetch all the category names once, instead of here it will 5 db calls if there are 5 categories selected, just to get the names of all the 5 categories.

I generally see template tags causing hidden db queries.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense! I was actually trying to figure out if I could cache the result of the initial query somehow but popping the login into view.py might make things easier.

@frjo
Copy link
Member

frjo commented Feb 21, 2024

@theskumar Would be really nice if you can improve the performance of the category query. I can see how this filter can be useful for staff.

@wes-otf
Copy link
Contributor Author

wes-otf commented Feb 21, 2024

that last push should be a lot more efficient ^

Copy link
Member

@theskumar theskumar left a comment

Choose a reason for hiding this comment

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

💯

@frjo frjo merged commit 30e8b39 into main Feb 22, 2024
10 checks passed
@theskumar theskumar deleted the fix/add-category-label-beta-all branch March 9, 2024 06:15
wes-otf added a commit that referenced this pull request May 7, 2024
Fixes #3741. Adds a new tag to get the label of the category from the ID
wes-otf added a commit that referenced this pull request May 8, 2024
Fixes #3741. Adds a new tag to get the label of the category from the ID
Vldln pushed a commit to equalitie/hypha that referenced this pull request May 28, 2024
Fixes HyphaApp#3741. Adds a new tag to get the label of the category from the ID
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Bugs! Things that are broken :-/ Type: Patch Mini change, used in release drafter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Beta submissions view categories label is rendered as an ID rather than a title
3 participants