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

Changes to report links and magazine report fix #320

Merged
merged 1 commit into from
Nov 30, 2023

Conversation

e-five256
Copy link
Member

@e-five256 e-five256 commented Nov 30, 2023

  • change report links to go to pending route
  • move pending to far left and all to far right
  • remove non-functional filter
  • fix duplicate where clause

I found the default page landing on all reports to be fairly useless, generally I think you're going to care about pending reports rather than all reports ever made. Instead of changing the default in routing (which I think would change say /reports to be pending and /reports/all to exist which might break history), I changed the links that go to reports to go to /pending instead. I also rearranged the menu items to be: Pending, Approved, Rejected, All

This also fixes a 500 error when viewing magazine reports for accepted/pending/rejected that I noticed while making this change

image

move pending to far left and all to far right
remove non-functional filter
fix duplicate where clause
@e-five256 e-five256 added bug Something isn't working frontend Visual issues, improvements, bugs or other aspects relating mostly to the front end labels Nov 30, 2023
@@ -64,33 +65,4 @@
</a>
</li>
</menu>
{% if is_route_name('magazine_panel_reports') %}
Copy link
Member Author

@e-five256 e-five256 Nov 30, 2023

Choose a reason for hiding this comment

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

this was unfunctional as far as I could tell. I didn't even notice it had existed before, but clicking the links does nothing ( I failed to get the label in the screenshot but it says "Filter by time" which isn't related to the menu items either)

image

Copy link
Member

@nobodyatroot nobodyatroot left a comment

Choose a reason for hiding this comment

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

Verified using kbin.run, looks good to me.

@e-five256 e-five256 merged commit 44b5b2c into main Nov 30, 2023
7 checks passed
@e-five256 e-five256 deleted the e5/report-pending-default branch November 30, 2023 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working frontend Visual issues, improvements, bugs or other aspects relating mostly to the front end
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants