-
Notifications
You must be signed in to change notification settings - Fork 127
feat: Allow Wikimedia users to register without email #1041
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
base: enext
Are you sure you want to change the base?
feat: Allow Wikimedia users to register without email #1041
Conversation
Reviewer's GuideThis PR enables Wikimedia OAuth users without an email by marking them with a new flag, generating placeholder emails, adapting the social auth flow, skipping notifications to placeholder addresses, and making email fields optional in relevant forms. Entity relationship diagram for User model and Wikimedia OAutherDiagram
USER {
string fullname
string wikimedia_username
boolean is_wikimedia_user
string email
}
SOCIALACCOUNT {
string provider
json extra_data
}
USER ||--o{ SOCIALACCOUNT : has
SOCIALACCOUNT }o--|| USER : belongs_to
Class diagram for updated User model with Wikimedia supportclassDiagram
class User {
+fullname: CharField
+wikimedia_username: CharField
+is_wikimedia_user: BooleanField
+is_active: BooleanField
+is_staff: BooleanField
+date_joined: DateTimeField
+send_security_notice(messages, email=None)
+send_password_reset(request)
+reset_password(event, user=None, mail_text=None, orga=False)
+change_password(new_password)
}
User <|-- UserSettingsForm
User <|-- get_or_create_user
User <|-- is_wikimedia_user
User <|-- get_or_create_email_for_wikimedia_user
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Abstract the '@wikimedia.local' placeholder domain into a configurable setting or constant to avoid hardcoding it in multiple places.
- Ensure placeholder emails remain unique (e.g. by appending a timestamp or random suffix) to prevent collisions for users sharing the same Wikimedia username.
- DRY up the repeated skip-email logic by extracting it into a decorator or helper so you don’t need to repeat the same
if endswith('@wikimedia.local')checks.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Abstract the '@wikimedia.local' placeholder domain into a configurable setting or constant to avoid hardcoding it in multiple places.
- Ensure placeholder emails remain unique (e.g. by appending a timestamp or random suffix) to prevent collisions for users sharing the same Wikimedia username.
- DRY up the repeated skip-email logic by extracting it into a decorator or helper so you don’t need to repeat the same `if endswith('@wikimedia.local')` checks.
## Individual Comments
### Comment 1
<location> `app/eventyay/plugins/socialauth/views.py:106-107` </location>
<code_context>
wikimedia_username = extra_data.get('username', extra_data.get('realname', ''))
+ # Generate placeholder email for Wikimedia users without email
+ final_email = get_or_create_email_for_wikimedia_user(wikimedia_username, email)
+
user, created = User.objects.get_or_create(
</code_context>
<issue_to_address>
**suggestion:** Consider handling empty Wikimedia usernames when generating placeholder emails.
Currently, an empty wikimedia_username results in a placeholder email missing the username part. Please add a check to ensure the username is not empty before generating the email.
```suggestion
# Generate placeholder email for Wikimedia users without email
if wikimedia_username:
final_email = get_or_create_email_for_wikimedia_user(wikimedia_username, email)
else:
final_email = email
```
</issue_to_address>
### Comment 2
<location> `app/eventyay/person/utils.py:15` </location>
<code_context>
+ """
+ if email and email.strip():
+ return email
+ return f"{username}@wikimedia.local"
+
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Placeholder email generation does not sanitize Wikimedia usernames.
Usernames with spaces or special characters could result in invalid email addresses. Please sanitize the username before constructing the email.
</issue_to_address>
### Comment 3
<location> `app/eventyay/base/forms/user.py:67-70` </location>
<code_context>
def __init__(self, *args, **kwargs):
self.user = kwargs.pop('user')
super().__init__(*args, **kwargs)
# Make email optional for Wikimedia users
if not is_wikimedia_user(self.user):
self.fields['email'].required = True
else:
self.fields['email'].required = False
self.fields['wikimedia_username'].widget.attrs['readonly'] = True
if self.user.auth_backend != 'native':
del self.fields['old_pw']
del self.fields['new_pw']
del self.fields['new_pw_repeat']
self.fields['email'].disabled = True
</code_context>
<issue_to_address>
**suggestion (code-quality):** We've found these issues:
- Replace if statement with if expression ([`assign-if-exp`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/assign-if-exp/))
- Simplify boolean if expression ([`boolean-if-exp-identity`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/boolean-if-exp-identity/))
- Remove unnecessary casts to int, str, float or bool ([`remove-unnecessary-cast`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/remove-unnecessary-cast/))
```suggestion
self.fields['email'].required = not is_wikimedia_user(self.user)
```
</issue_to_address>
### Comment 4
<location> `app/eventyay/person/utils.py:13-15` </location>
<code_context>
def get_or_create_email_for_wikimedia_user(username, email=None):
"""
Generate an email for Wikimedia users.
If email exists, use it. Otherwise, create a placeholder.
Args:
username (str): Wikimedia username
email (str): Email from Wikimedia profile (can be None)
Returns:
str: Email address
"""
if email and email.strip():
return email
return f"{username}@wikimedia.local"
</code_context>
<issue_to_address>
**suggestion (code-quality):** We've found these issues:
- Lift code into else after jump in control flow ([`reintroduce-else`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/reintroduce-else/))
- Replace if statement with if expression ([`assign-if-exp`](https://docs.sourcery.ai/Reference/Default-Rules/refactorings/assign-if-exp/))
```suggestion
return email if email and email.strip() else f"{username}@wikimedia.local"
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
mariobehling
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, I’ve run an AI-assisted review on this PR to catch structural and consistency issues.
The feature direction looks fine, but there are several must-fix items before merge — please review carefully.
🧩 1) UserSettingsForm – redundant required assignment
You set self.fields['email'].required = True and then re-set it conditionally.
Please simplify to a single clear line:
self.fields['email'].required = not is_wikimedia_user(self.user)This avoids confusing double assignment.
🧩 2) Placeholder email – no sanitization
get_or_create_email_for_wikimedia_user builds f"{username}@wikimedia.local" directly.
This can yield invalid or colliding addresses (spaces, Unicode, punctuation).
Add sanitization:
- lowercase, strip, replace whitespace →
. - remove disallowed chars
- optionally append Wikimedia user ID to ensure uniqueness.
🧩 3) Empty username edge case
If both username and realname are missing, wikimedia_username becomes '', producing @wikimedia.local.
Guard this explicitly—either:
- fail with a user-friendly error, or
- generate a deterministic surrogate (
wm-{social_id}@wikimedia.local).
🧩 4) User lookup by placeholder email → collision risk
User.objects.get_or_create(email=final_email) relies on the synthetic email for uniqueness.
Different users could collide or one user could later switch to a real email.
Either:
- guarantee unique placeholders (e.g., include Wikimedia ID), or
- key on
wikimedia_username + providerinstead of email.
🧩 5) i18n missing for new fields
verbose_name=('Wikimedia username') and is_wikimedia_user should be wrapped in _() for translation consistency.
🧩 6) Mail suppression logic duplicated
Several methods (send_security_notice, send_password_reset, reset_password, change_password) repeat
if self.email.endswith('@wikimedia.local'):
returnExtract a helper like is_placeholder_email(email) and reuse across all mail-sending paths.
🧩 7) Migration index check
Migration 0005_user_is_wikimedia_user depends on 0004_....
Please confirm no newer migration was added on enext since then — rebase or renumber if necessary.
✅ Next steps
- Address all seven items above.
- Re-run tests after sanitization and form updates.
- Add a small test case for the new utils once you introduce them.
💡 Note: This review used AI assistance for consistency and code-quality scanning.
I recommend you also use an AI tool (e.g. ChatGPT or Sourcery AI) to review your future contributions — paste your code or PR URL and ask it to spot logical and structural issues. It’s very effective for edge cases like the placeholder-email flow.
7fb880f to
8935b93
Compare
mariobehling
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The conflicts of this PR need to be resolved before it can be further reviewed.
612ffb9 to
bd64b99
Compare
Hi @mariobehling ! i've rebased this PR on the latest enext branch. GitHub is still showing a conflict but when I test the merge locally, it completes successfully with no conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for Wikimedia OAuth users with placeholder email addresses, allowing users who don't provide real email addresses to use the platform without receiving unwanted notifications.
Key changes:
- Added utilities to generate and identify placeholder Wikimedia emails
- Modified user authentication flow to support Wikimedia users without real emails
- Added database field to track Wikimedia user status
- Updated form validation to make email optional for Wikimedia users
Reviewed Changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/person/test_wikimedia_utils.py | Comprehensive test coverage for Wikimedia email utility functions |
| app/eventyay/person/utils.py | Core utility functions for generating placeholder emails and identifying Wikimedia users |
| app/eventyay/plugins/socialauth/views.py | Updated OAuth flow to use wikimedia_username as primary lookup key and generate placeholder emails |
| app/eventyay/presale/forms/checkout.py | Made email field optional for Wikimedia users in checkout forms |
| app/eventyay/base/models/auth.py | Added is_wikimedia_user field and guards to prevent sending emails to placeholder addresses |
| app/eventyay/base/forms/user.py | Made email field optional for Wikimedia users in user settings |
| app/eventyay/base/migrations/0005_user_is_wikimedia_user.py | Database migration to add is_wikimedia_user field |
| app/eventyay/config/urls.py | Added socialauth plugin URLs to main URL configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mariobehling
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. Please check Copilot comments. Also, we will reset the database migrations through another PR in the upcoming days. So, this would require a change in this regard as well.
aa0b1d3 to
45ec22a
Compare
mariobehling
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Aqil-Ahmad Please take into account the changes mentioned here #1013 (comment)
There is an issue with the placeholder requirement. We actually do not need it.
Regarding the functionality of this feature. We need to check in detail.
Please make the adjustments based on the comments.
thanks for the feedback! |
Description
Allows users to register for tickets and speaker accounts using Wikimedia OAuth, even when their Wikimedia profile doesn't have an associated email address.
Changes
Closes
Fixes #1013
Summary by Sourcery
Allow Wikimedia OAuth users to register without an email by generating placeholder email addresses, adding an is_wikimedia_user flag to the user model, suppressing email notifications for placeholder accounts, and making email optional in relevant forms.
New Features:
Enhancements: