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

Make email field in user form required #449

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lunes_cms/cms/admins/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@
"GroupAPIKeyAdmin",
"FeedbackAdmin",
"SponsorAdmin",
"UserAdmin",
Copy link
Member

Choose a reason for hiding this comment

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

Changing __all__ without importing the UserAdmin here is completely useless. You have to insert from .user_admin import UserAdmin in this file if you want to enable other modules to do a from lunes_cms.cms.admins import UserAdmin.

]
7 changes: 7 additions & 0 deletions lunes_cms/cms/admins/user_admin.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
from django.contrib import admin

from ..models import RegisterUserForm
Copy link
Member

Choose a reason for hiding this comment

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

What does this line do? 1. There is no RegisterUserForm in our code base and 2. forms are forms and not models.



class UserAdmin(admin.ModelAdmin):
Copy link
Member

Choose a reason for hiding this comment

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

New admins also have to be registered in lunes_cms/cms/admin.py, otherwise they are just a few lines of code that are never executed.

fields = ("password", "email", "username")
38 changes: 38 additions & 0 deletions lunes_cms/cms/migrations/0010_user.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# Generated by Django 3.2.16 on 2023-03-07 16:16

from django.db import migrations, models


class Migration(migrations.Migration):
Copy link
Member

Choose a reason for hiding this comment

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

Just adding the autogenerated migration won't magically migrate all existing users to the new model.

dependencies = [
("cms", "0009_sponsor"),
]

operations = [
migrations.CreateModel(
name="User",
fields=[
(
"id",
models.AutoField(
auto_created=True,
primary_key=True,
serialize=False,
verbose_name="ID",
),
),
("password", models.CharField(max_length=128, verbose_name="password")),
(
"last_login",
models.DateTimeField(
blank=True, null=True, verbose_name="last login"
),
),
("username", models.CharField(default="", max_length=30, unique=True)),
("email", models.EmailField(default="", max_length=254)),
],
options={
"abstract": False,
},
),
]
2 changes: 2 additions & 0 deletions lunes_cms/cms/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from .group_api_key import GroupAPIKey
from .feedback import Feedback
from .sponsor import Sponsor
from .user import User
from . import group, content_type

__all__ = [
Expand All @@ -22,4 +23,5 @@
"GroupAPIKey",
"Feedback",
"Sponsor",
"User",
]
28 changes: 28 additions & 0 deletions lunes_cms/cms/models/user.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
from django.db import models
from django.contrib.auth.models import BaseUserManager, AbstractBaseUser


class UserManager(BaseUserManager):
def create_user(self, username, email, password):
if not email:
raise ValueError("_('Users must have an email address')")
if not password:
raise ValueError("_('Users must have a password')")
user = self.model(email=self.normalize_email(email))
user.set_username(username)
user.set_password(password)
user.save()
return user
Comment on lines +5 to +15
Copy link
Member

Choose a reason for hiding this comment

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

No sure if we need a custom manager for this. The main concern is the UI in the CMS, not the createsuperuser management command.



class User(AbstractBaseUser):
"""
user model
"""

username = models.CharField(default="", unique=True, max_length=30)
email = models.EmailField(default="")
Comment on lines +23 to +24
Copy link
Member

Choose a reason for hiding this comment

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

A default value of "" sounds pretty unreasonable to me.


USERNAME_FIELD = "username"
REQUIRED_FIELDS = ["email", "password"]
Copy link
Member

Choose a reason for hiding this comment

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

REQUIRED_FIELDS must contain all required fields on your user model, but should not contain the USERNAME_FIELD or password as these fields will always be prompted for.

https://docs.djangoproject.com/en/4.1/topics/auth/customizing/#django.contrib.auth.models.CustomUser.REQUIRED_FIELDS

objects = UserManager()
2 changes: 2 additions & 0 deletions lunes_cms/core/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,8 @@
},
]

# AUTH_USER_MODEL = 'cms.User'


########################
# INTERNATIONALIZATION #
Expand Down
10 changes: 5 additions & 5 deletions lunes_cms/locale/de/LC_MESSAGES/django.po
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ msgid ""
msgstr ""
"Project-Id-Version: PACKAGE VERSION\n"
"Report-Msgid-Bugs-To: \n"
"POT-Creation-Date: 2023-02-20 20:24+0000\n"
"POT-Creation-Date: 2023-03-07 16:16+0000\n"
"PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n"
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
"Language-Team: LANGUAGE <[email protected]>\n"
Expand Down Expand Up @@ -494,19 +494,19 @@ msgstr "Datei zu groß! Max. 5 MB"
msgid "Only use one file extension!"
msgstr "Nur maximal ein Dateityp erlaubt!"

#: core/settings.py:203
#: core/settings.py:205
msgid "English"
msgstr "Englisch"

#: core/settings.py:204
#: core/settings.py:206
msgid "German"
msgstr "Deutsch"

#: core/settings.py:446 core/settings.py:447 core/settings.py:449
#: core/settings.py:448 core/settings.py:449 core/settings.py:451
msgid "Lunes Administration"
msgstr "Lunes-Verwaltung"

#: core/settings.py:448
#: core/settings.py:450
msgid "Welcome to the vocabulary management of Lunes!"
msgstr "Willkommen bei der Vokabelverwaltung von Lunes!"

Expand Down