Skip to content

Conversation

@ihsaan-ullah
Copy link
Collaborator

@ihsaan-ullah ihsaan-ullah commented Apr 25, 2023

@ mention of reviewers

@Didayolo @bbearce @dtuantran

A brief description of the purpose of the changes contained in this PR.

Now user can use either username or email to login

Checklist

  • Code review by me
  • Hand tested by me
  • I'm proud of my work
  • Code review by reviewer
  • Hand tested by reviewer
  • CircleCi tests are passing
  • Ready to merge

@Didayolo
Copy link
Member

@bbearce Could you please review this PR?

@Didayolo
Copy link
Member

@ihsaan-ullah @bbearce I've tested it locally and it works like a charm.

@bbearce
Copy link
Collaborator

bbearce commented May 1, 2023

Checking the 6 new files to see if the design fits normal django design patterns.

Sources for patterns:

  1. src/apps/profiles/backends.py - Ihsaan makes an email authentication backend class like all examples show.
  2. src/apps/profiles/forms.py - We need a custom form inherited from Django's built in forms. We were using a default one but now we need to edit that a bit. This seems on point with the tutorials.
    • Question for the team: Do we need any validation? I thought so for username but only previously correct usernames are allowed during sign up so we don't need to check them for log in as only a correct one would theoretically work.
  3. src/apps/profiles/urls_accounts.py - turn login view from default to an actual one.
  4. src/apps/profiles/views.py - Looks good but I couldn't log in with username anymore. I mad a new commit that allows either.
  5. src/settings/base.py - As per notes, profiles.backends.EmailAuthenticationBackend needs to be added to AUTHENTICATION_BACKENDS
  6. src/templates/registration/login.html - simple update letting user know they can use either username or email.

@Didayolo Didayolo force-pushed the login_email_username branch from 329d3ca to 42bfc5c Compare May 2, 2023 15:42
@Didayolo Didayolo merged commit 672acdc into develop May 2, 2023
@Didayolo Didayolo deleted the login_email_username branch May 2, 2023 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants