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

Add a custom user model to the testapp #190

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Conversation

rhysyngsun
Copy link
Collaborator

@rhysyngsun rhysyngsun commented Feb 26, 2025

Review #189 first, as this PR is based on it.

What are the relevant tickets?

Part of https://github.com/mitodl/hq/issues/6584

Description (What does it do?)

The implementation for #187 expects the user model to be customized and contain the SCIM fields, so we need to add a custom user model.

How can this be tested?

  • Tests should pass
  • Run uv run testapp/manage.py showmigrations, you should see the expected output.
  • Note: you may need to drop your DB, since this is only for tests I didn't think it was worth having to deal with 3-4 migrations/PRs

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Hello @rhysyngsun, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request addresses the need for a custom user model in the testapp application. It introduces a users app with a custom User model extending Django's AbstractUser. The changes also involve refactoring the testapp application to rename it to main, and updating all references to the old name to reflect the new name. This includes settings files, imports, and other configurations. Additionally, the PR includes updates to the test suite to accommodate these changes and adds a new fixture for loading data fixtures.

Highlights

  • Custom User Model: Adds a users app with a custom User model extending Django's AbstractUser.
  • Application Refactor: Renames the testapp application to main and updates all relevant references.
  • Settings Updates: Modifies settings files to include the new users app and update references to the renamed main app.
  • Test Suite Updates: Adjusts the test suite to reflect the application refactor and custom user model.
  • New Fixture: Adds a new pytest fixture for loading data fixtures from JSON files.

Changelog

Click here to see the changelog
  • .secrets.baseline
    • Updates the .secrets.baseline file to reflect changes in file paths and potential secret keywords.
  • conftest.py
    • Adds imports for json, contextmanager, and Path.
    • Adds new pytest fixtures open_data_fixture_file and load_data_fixture_json for loading data fixtures.
  • pyproject.toml
    • Updates DJANGO_SETTINGS_MODULE to main.settings.test.
    • Updates pythonpath to include testapp.
    • Updates django_settings_module to main.settings.test.
  • testapp/main/README.md
    • Updates imports from testapp.factories to main.factories.
    • Updates imports from testapp.messages to main.messages.
  • testapp/main/admin.py
    • Updates imports from testapp.models to main.models.
  • testapp/main/factories.py
    • Updates imports from testapp.models to main.models.
  • testapp/main/migrations/0001_initial.py
    • Updates ManyToManyField to point to main.SecondLevel2.
    • Updates ForeignKey to point to main.SecondLevel1.
  • testapp/main/migrations/0002_auditabletestmodel.py
    • Updates dependency from testapp to main.
  • testapp/main/migrations/0003_auditabletestmodelaudit.py
    • Updates dependency from testapp to main.
    • Updates ForeignKey to point to main.auditabletestmodel.
  • testapp/main/settings/dev.py
    • Updates imports from testapp.settings.shared to main.settings.shared.
    • Updates MITOL_DIGITAL_CREDENTIALS_BUILD_CREDENTIAL_FUNC to mitol.digitalcredentials.main.integration.build_credential.
  • testapp/main/settings/example.dev.py
    • Updates imports from testapp.settings.shared to main.settings.shared.
    • Updates MITOL_DIGITAL_CREDENTIALS_BUILD_CREDENTIAL_FUNC to main.integration.build_credential.
    • Updates MITOL_MAIL_MESSAGE_CLASSES to main.messages.SampleMessage.
  • testapp/main/settings/shared.py
    • Updates Django settings for main project.
    • Adds AUTH_USER_MODEL = "users.User".
    • Adds users to INSTALLED_APPS.
    • Updates ROOT_URLCONF to main.urls.
    • Updates DIRS in TEMPLATES to BASE_DIR + "/main/templates/".
    • Updates WSGI_APPLICATION to main.wsgi.application.
  • testapp/main/settings/test.py
    • Updates imports from testapp.settings.shared to main.settings.shared.
    • Updates MITOL_DIGITAL_CREDENTIALS_BUILD_CREDENTIAL_FUNC to main.integration.build_credential.
    • Updates MITOL_MAIL_MESSAGE_CLASSES to main.messages.SampleMessage.
  • testapp/main/urls.py
    • Updates imports from testapp.views to main.views.
  • testapp/main/views.py
    • Updates imports from testapp.models to main.models.
  • testapp/main/wsgi.py
    • Updates DJANGO_SETTINGS_MODULE to main.settings.dev.
  • testapp/manage.py
    • Updates DJANGO_SETTINGS_MODULE to main.settings.dev.
  • testapp/users/admin.py
    • Adds an empty file to register models.
  • testapp/users/apps.py
    • Creates a UsersConfig class.
  • testapp/users/migrations/0001_initial.py
    • Creates the initial migration for the users app.
  • testapp/users/models.py
    • Creates a custom User model extending AbstractUser.
  • testapp/users/tests.py
    • Adds an empty file for tests.
  • testapp/users/views.py
    • Adds an empty file for views.
  • tests/common/test_apps.py
    • Updates imports from testapp to main.
  • tests/common/test_envs.py
    • Updates DJANGO_SETTINGS_MODULE to main.settings.test.
  • tests/common/test_models.py
    • Updates imports from testapp.models to use models from main.
  • tests/digitalcredentials/test_backend.py
    • Updates imports from testapp.factories to main.factories.
  • tests/digitalcredentials/test_models.py
    • Updates imports from testapp.factories to main.factories.
  • tests/digitalcredentials/test_serializers.py
    • Removes imports from testapp.factories and imports from main.factories.
  • tests/digitalcredentials/test_views.py
    • Updates imports from testapp.factories to main.factories.
  • tests/google_sheets/test_views.py
    • Updates imports from testapp.utils to main.utils.
  • tests/google_sheets_deferrals/test_api.py
    • Removes import os.
    • Uses open_data_fixture_file fixture to open the data file.
  • tests/google_sheets_refunds/test_api.py
    • Removes import os.
    • Uses open_data_fixture_file fixture to open the data file.
  • tests/hubspot_api/test_api.py
    • Imports get_user_model from django.contrib.auth.
    • Uses get_user_model() to get the User model.
  • tests/mail/testapp/test_messages.py
    • Updates imports from testapp.messages to main.messages.
  • tests/payment_gateway/api/test_cybersource.py
    • Removes import os.
    • Imports factories from main.factories instead of testapp.factories.
    • Uses load_data_fixture_json fixture to load the data file.
  • uv.lock
    • Updates the version of mitol-django-transcoding from 2025.2.22 to 2025.2.25.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.


Did you know?

Django's ORM (Object-Relational Mapper) allows developers to interact with databases using Python code, abstracting away the complexities of SQL.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a custom user model to the testapp, which is necessary for the implementation in PR #187. The changes involve updating various files to reflect the new user model and its location. Overall, the changes seem well-structured and necessary for the project's evolution. However, there are a few points that could be improved for better maintainability and clarity.

Summary of Findings

  • Secret Keyword in .secrets.baseline: The .secrets.baseline file contains entries for 'Secret Keyword' that should be reviewed to ensure no actual secrets are exposed.
  • Module naming consistency: In several files, the module path 'testapp' is being replaced with 'main'. It's important to ensure this change is consistent across the entire application to avoid confusion.
  • File path in open_data_fixture_file: The file path used in the open_data_fixture_file fixture in conftest.py could be simplified for better readability and maintainability.

Assessment

The pull request is mostly in good shape and introduces necessary changes for the custom user model. However, there are a few medium severity issues that should be addressed before merging. Specifically, the secret keyword in .secrets.baseline should be verified, module naming consistency should be ensured, and the file path in open_data_fixture_file should be simplified. After addressing these comments, the code should be reviewed and approved by others before merging.

@rhysyngsun rhysyngsun changed the title Add a cusotm user model to the testapp Add a custom user model to the testapp Feb 26, 2025
@jkachel jkachel self-assigned this Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants