Fix inconsistent sidebar hover effects across admin panels #1316
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
Reviewer's GuideImplements comprehensive Wikimedia OAuth support by introducing an is_wikimedia_user flag and unique wikimedia_username, refactors user creation logic to guard and branch on social authentication, adjusts model and form behavior around optional emails, adds utilities, tests, and a migration, and unifies sidebar hover styling. Sequence diagram for user creation via Wikimedia OAuthsequenceDiagram
actor User
participant "Django Request"
participant "SocialAccount"
participant "User Model"
User->>"Django Request": Login via Wikimedia OAuth
"Django Request"->>"SocialAccount": Get latest Wikimedia account
"Django Request"->>"User Model": get_or_create(wikimedia_username, is_wikimedia_user)
"User Model"-->>"Django Request": Return user instance
"Django Request"-->>User: Login complete
Sequence diagram for optional email in forms for Wikimedia userssequenceDiagram
actor User
participant "Checkout/UserSettings Form"
participant "is_wikimedia_user()"
User->>"Checkout/UserSettings Form": Access form
"Checkout/UserSettings Form"->>"is_wikimedia_user()": Check user type
"is_wikimedia_user()"-->>"Checkout/UserSettings Form": True/False
"Checkout/UserSettings Form"-->>User: Email field required/optional
Entity relationship diagram for updated User model (Wikimedia support)erDiagram
USER {
string email
string fullname
string wikimedia_username "(unique)"
boolean is_wikimedia_user
boolean is_active
boolean is_staff
datetime date_joined
}
USER ||--o{ SOCIALACCOUNT : has
SOCIALACCOUNT {
string provider
string uid
json extra_data
}
Class diagram for updated User model and is_wikimedia_user utilityclassDiagram
class User {
email: str
fullname: str
wikimedia_username: str
is_wikimedia_user: bool
is_active: bool
is_staff: bool
date_joined: datetime
get_full_name()
send_security_notice(messages, email=None)
send_password_reset(request)
reset_password(event, user=None, mail_text=None, orga=False)
change_password(new_password)
}
class is_wikimedia_user_user {
<<function>>
Returns: bool
}
User <|-- is_wikimedia_user_user
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- You have repeated the is_wikimedia_user check in multiple form classes – consider extracting this into a shared mixin or helper to DRY up the code.
- The get_or_create_user logic splits on email vs. wikimedia_username but doesn’t handle cases where both identifiers map to the same person and could create duplicate accounts – consider adding explicit merge or conflict resolution.
- The CSS hover selectors for active and non-active nav-links are scattered across rules – consolidating them into a single selector or mixin will help maintain consistency and avoid specificity issues.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- You have repeated the is_wikimedia_user check in multiple form classes – consider extracting this into a shared mixin or helper to DRY up the code.
- The get_or_create_user logic splits on email vs. wikimedia_username but doesn’t handle cases where both identifiers map to the same person and could create duplicate accounts – consider adding explicit merge or conflict resolution.
- The CSS hover selectors for active and non-active nav-links are scattered across rules – consolidating them into a single selector or mixin will help maintain consistency and avoid specificity issues.
## Individual Comments
### Comment 1
<location> `app/eventyay/base/models/auth.py:166` </location>
<code_context>
)
fullname = models.CharField(max_length=255, blank=True, null=True, verbose_name=_('Full name'))
- wikimedia_username = models.CharField(max_length=255, blank=True, null=True, verbose_name=('Wikimedia username'))
+ wikimedia_username = models.CharField(max_length=255, blank=True, null=True, unique=True, verbose_name=_('Wikimedia username'))
+ is_wikimedia_user = models.BooleanField(default=False, verbose_name=_('Is Wikimedia user'))
is_active = models.BooleanField(default=True, verbose_name=_('Is active'))
</code_context>
<issue_to_address>
**issue (bug_risk):** Adding unique=True to a nullable CharField can cause issues with multiple NULLs.
Consider making the field non-nullable or adding a custom constraint if you require strict uniqueness, as multiple NULLs may bypass the unique constraint.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Aqil-Ahmad
force-pushed
the
fix/hover
branch
from
0530a28 to
790ede3
Compare
Author
|
Hi @mariobehling! i created the issue and opened this PR for it, but i'm not fully sure if these changes are required. could you please review and advise? |
Copilot finished reviewing on behalf of
mariobehling
November 20, 2025 02:07
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes inconsistent sidebar hover effects across admin panels by standardizing the hover behavior. The Talk panel previously showed darker text/icon colors on hover while Common and Tickets panels only showed gray background. Additionally, the dashboard/search icon (#nav-search) lacked the hover darken effect.
- Removed inconsistent hover color changes from nav-link elements to match Common and Tickets panels
- Added hover color change to the dashboard/search icon for consistency
- Simplified hover state rules by consolidating active and non-active states
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mariobehling
requested changes
Nov 23, 2025
Member
mariobehling
left a comment
mariobehling
left a comment
There was a problem hiding this comment.
Please provide a short video to show what you changed.
Author
hi @mariobehling! i've attached the video in the pr description. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes : #1315
Description
the Talk panel's sidebar menu items showed darker text/icon colors on hover, while Common and Tickets panels only showed gray background. This created an inconsistent user experience. also the dashboard icon didn't show darken effect in talk panel's sidebar unlike the others.
Summary by Sourcery
Extend User model and authentication flow to better support Wikimedia OAuth users with unique identifiers and adaptive form and email handling, while also refining admin sidebar hover styling for consistency.
New Features:
Enhancements:
Build:
Tests:
Recording.2025-11-23.100401.mp4