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

replace self-made sortable model with jazzband django-admin-sortable #1698

Merged
merged 2 commits into from
Sep 29, 2023

Conversation

Bomme
Copy link
Contributor

@Bomme Bomme commented Sep 27, 2023

Description
I'm proposing to replace our implementation of a sortable model with the jazzband package django-admin-sortable

The idea came, after our linter discovered that the current code was broken and I tried to wrap my head around what the code is doing.

Deployment steps:

  1. migrate

@Bomme Bomme requested a review from alastair September 27, 2023 19:20
pass
except ModelClass.DoesNotExist:
pass
class OrderedModel(SortableMixin):
Copy link
Member

Choose a reason for hiding this comment

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

I think that we found that there was just one model that was sortable, right? Licenses is sortable but we can't find where that logic is used. In that case, I'd just add SortableMixin and the order field directly to the Forum model

Copy link
Contributor Author

Choose a reason for hiding this comment

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

right, alternatively we could make it sortable in admin with adding LicenseAdmin(SortableAdmin).
What would happen if we remove the ordering from the License model (table)?

Copy link
Member

Choose a reason for hiding this comment

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

I'd double-check with @ffont if we need to order licenses, and if not we remove it.

Copy link
Member

Choose a reason for hiding this comment

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

I think sortable is only used in forums because, well, we wanted a custom ordering to display forums. But this is not needed for Licenses so no need to add it to them.

requirements.in Outdated Show resolved Hide resolved
@Bomme Bomme requested a review from ffont September 28, 2023 08:12
@ffont ffont merged commit fcfe1e8 into master Sep 29, 2023
1 check passed
@ffont ffont deleted the sortable_model branch September 29, 2023 08:47
@ffont ffont restored the sortable_model branch October 24, 2023 19:17
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