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

Support reordering sub flair #284

Closed
wants to merge 2 commits into from

Conversation

steadyember
Copy link
Contributor

@steadyember steadyember commented Jan 1, 2021

Closes #265

Copy link
Member

@Polsaker Polsaker left a comment

Choose a reason for hiding this comment

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

Looks okay apart from the migration error, though a more user-friendly way to order it would be with drag-and-drop (like in /settings/subs). Only being able to move a flair to the top/bottom might become a hassle for subs with tons of flairs

"""Write your migrations here."""
migrator.add_fields(
migrator.orm['sub_flair'],
display_order=pw.IntegerField(constraints=[SQL("DEFAULT 1")]),
Copy link
Member

Choose a reason for hiding this comment

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

This shouild be like this:

Suggested change
display_order=pw.IntegerField(constraints=[SQL("DEFAULT 1")]),
display_order=pw.IntegerField(default=1),

Otherwise migrations will fail with ValueError: display_order is not null but has no default

@Polsaker
Copy link
Member

Polsaker commented Apr 6, 2021

Four months have passed. I'm considering this PR as abandoned. Please create a new one if you still want your changes to get merged.

@Polsaker Polsaker closed this Apr 6, 2021
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.

Ability to reorder flair
2 participants