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

Clean up logic for header tabs #588

Closed
wants to merge 3 commits into from
Closed

Conversation

Tammo-Feldmann
Copy link
Contributor

What I'm changing

I started this cleanup project when I ran into an issue with the object headers where published drafts where not showing up correctly. I refactored and simplified the logic around the arguments for the header tab.

Before

header_tabs_before

After

header_tabs_after

has_progress_draft = (
Change.objects.exclude(status=Change.Statuses.PUBLISHED)
.filter(Q(uuid=change.uuid) | Q(model_instance_uuid=change.uuid))
has_published_draft = (
Copy link
Member

Choose a reason for hiding this comment

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

Purely a style suggestion, but has_published_draft is an awful lot like has_progress_drafts.

Can we move those queries to be near eachother (ie either assigned to a variable up here or written inline in the returned dict)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me. Made the change that you requested.

app/admin_ui/views/doi.py Outdated Show resolved Hide resolved
Base automatically changed from feature/canonical-mi-workflow to dev November 30, 2023 19:04
@Tammo-Feldmann
Copy link
Contributor Author

This work was merged elsewhere and we don't need this PR anymore.

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.

2 participants