Skip to content

Admin page accessible from internal pages #755

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

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

TheStrgamer
Copy link
Contributor

@TheStrgamer TheStrgamer commented Mar 6, 2025

Proposed changes

Made the administration page accessible from the dropdown menu in internal pages, so you don't have to go back to public pages to access it.

Checklist

(If any of the points are not relevant, mark them as checked)

  • Remembered to run the makemigrations, makemessages and compilemessages management commands and committed any changes that should be included in this PR
  • Created tests that fail without the changes, if relevant/possible
  • Manually tested that the website UI works as intended with different device layouts
    • (Most common is to test with typical screen sizes for mobile (320-425 px), tablet (768 px) and desktop (1024+ px), which can easily be done with your browser's dev tools)
  • Manually tested that everything works as intended when logged in as different users locally
    • (This can be e.g. anonymous users (i.e. not being logged in), "normal" non-member users, members of different committees, and superusers)
  • Made sure that your code conforms to the code style guides
    • (It's not intended that you read through this whole document, but that you get yourself an overview over its contents, and that you use it as a reference guide / checklist while taking a second look at your code before opening a pull request)
  • Attempted to minimize the number of common code smells
    • (See the comment for the previous checkbox)
  • Added sufficient documentation - e.g. as comments, docstrings or in the README, if suitable
  • Added your changes to the "Unreleased" section of the changelog - mainly the changes that are of particular interest to users and/or developers, if any
  • Added a "Deployment notes" section above, if anything out of the ordinary should be done when deploying these changes to the server
  • Structured your commits reasonably

@make-bot make-bot bot added this to web Mar 6, 2025
@make-bot make-bot bot moved this to Ready for Review in web Mar 6, 2025
Copy link

codecov bot commented Mar 6, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.17%. Comparing base (5715a77) to head (8ed38da).
Report is 1 commits behind head on dev.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##              dev     #755   +/-   ##
=======================================
  Coverage   88.17%   88.17%           
=======================================
  Files         152      152           
  Lines        6198     6198           
=======================================
  Hits         5465     5465           
  Misses        733      733           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@TheStrgamer TheStrgamer changed the base branch from main to dev March 6, 2025 15:41
@ddabble ddabble added the ⚡Quick review⚡ This PR is short and can be reviewed quickly. label Mar 23, 2025
Comment on lines +41 to +43
<a class="item" href="{% host_url 'admin_panel' host 'main' %}">
<div class="text">{% translate "Administration" %}</div>
</a>
Copy link
Member

@ddabble ddabble Mar 24, 2025

Choose a reason for hiding this comment

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

From a UX perspective, I don't think this button should be in this menu, as it can easily be confused to mean "Administration of the internal pages (i.e. the site I'm currently on)" - given that we've been able to successfully create a mental separation between the public and internal site with our users, of course. And as it is now, it's just one extra click to get to the public pages, anyway :)

But if you all agree that the button should be there, I'd suggest wrapping it in the same check as is done on the public pages, so that unprivileged users are not met with a 403 page 🙂

Suggested change
<a class="item" href="{% host_url 'admin_panel' host 'main' %}">
<div class="text">{% translate "Administration" %}</div>
</a>
{% if user|can_view_admin_panel %}
<a class="item" href="{% host_url 'admin_panel' host 'main' %}">
<div class="text">{% translate "Administration" %}</div>
</a>
{% endif %}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⚡Quick review⚡ This PR is short and can be reviewed quickly.
Projects
Status: Ready for Review
Development

Successfully merging this pull request may close these issues.

2 participants