Fix user migrations for SCIM #2078

rhysyngsun · 2025-02-27T19:14:36Z

What are the relevant tickets?

N/A

Description (What does it do?)

This fixes an issue that came up in deployment to RC w/ existing data and updated_on not being nullable by default.

How can this be tested?

You should be able to pull the branch and run migrations without issue.

gemini-code-assist

Hello @rhysyngsun, I'm Gemini Code Assist¹! 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 an issue encountered during deployment to the RC environment where the updated_on field in the User model was not nullable by default, causing migration problems with existing data. The changes include making the updated_on field nullable, setting SCIM-related fields in the UserFactory, and adjusting the SCIM user search functionality to allow sorting by 'id'.

Highlights

Migration Fix: The updated_on field in the User model is now nullable to prevent migration issues with existing data.
SCIM Attributes in UserFactory: Added scim_external_id and scim_username attributes to the UserFactory for easier SCIM user creation during testing.
SCIM Search Sorting: The SCIM user search functionality now supports sorting by 'id' in addition to 'email' and 'username'.

Changelog

Click here to see the changelog

main/factories.py
- Imported SelfAttribute from factory.
- Added scim_external_id and scim_username fields to the UserFactory.
scim/views.py
- Set default sort_by to id.
- Allowed sorting by id in the SCIM user search.
- Removed conditional qs.order_by since it's always applied now.
scim/views_test.py
- Added a large_user_set fixture to create a large number of users for testing.
- Added test cases for sorting by 'id' in the user search tests.
- Modified test_user_search to use the large_user_set fixture and adjust sorting logic.
- Updated test_user_search to use user.scim_id and user.scim_external_id instead of user.profile.scim_id and user.profile.scim_external_id.
- Updated test_user_search to use user.updated_on and user.created_on instead of user.profile.updated_at and user.date_joined.
users/migrations/0004_add_scim_and_timestamp_fields.py
- Made the updated_on field nullable by setting null=True.
users/migrations/0005_set_user_scim_id.py
- Added a migration to revert the updated_on field to non-nullable.

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.

Trivia time!

What does SCIM stand for?

Click here for the answer

SCIM stands for System for Cross-domain Identity Management.

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. ↩

gemini-code-assist

Code Review

The pull request addresses an issue with user migrations for SCIM, specifically related to the updated_on field and sorting in the SCIM API. The changes include making the updated_on field nullable, setting SCIM IDs and timestamps in a data migration, and adjusting the SCIM user search endpoint. Overall, the changes seem reasonable and address the described problem.

Summary of Findings

Merge Readiness

The changes appear to address the described issue and include necessary adjustments for SCIM functionality. I am unable to directly approve the pull request, and others should review and approve this code before merging. Given the nature of the changes (migrations and API adjustments), it would be beneficial to have additional testing and verification, especially in a staging environment, before merging to production.

mbertrand · 2025-02-27T19:44:18Z

I had already run some of these migrations so reverted back to 0003: ./manage.py migrate users 0003_rename_user_table

Then ran docker compose up and got this migration error:

web-1  | Running migrations:
web-1  |   Applying users.0006_remove_auth_user_view...Traceback (most recent call last):
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/backends/utils.py", line 87, in _execute
web-1  |     return self.cursor.execute(sql)
web-1  |            ^^^^^^^^^^^^^^^^^^^^^^^^
web-1  | psycopg2.errors.UndefinedTable: view "auth_user" does not exist
web-1  | 
web-1  | 
web-1  | The above exception was the direct cause of the following exception:
web-1  | 
web-1  | Traceback (most recent call last):
web-1  |   File "/src/manage.py", line 29, in <module>
web-1  |     execute_from_command_line(sys.argv)
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line
web-1  |     utility.execute()
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/core/management/__init__.py", line 436, in execute
web-1  |     self.fetch_command(subcommand).run_from_argv(self.argv)
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/core/management/base.py", line 412, in run_from_argv
web-1  |     self.execute(*args, **cmd_options)
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/core/management/base.py", line 458, in execute
web-1  |     output = self.handle(*args, **options)
web-1  |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/core/management/base.py", line 106, in wrapper
web-1  |     res = handle_func(*args, **kwargs)
web-1  |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/core/management/commands/migrate.py", line 356, in handle
web-1  |     post_migrate_state = executor.migrate(
web-1  |                          ^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/migrations/executor.py", line 135, in migrate
web-1  |     state = self._migrate_all_forwards(
web-1  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/migrations/executor.py", line 167, in _migrate_all_forwards
web-1  |     state = self.apply_migration(
web-1  |             ^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/migrations/executor.py", line 252, in apply_migration
web-1  |     state = migration.apply(state, schema_editor)
web-1  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/migrations/migration.py", line 132, in apply
web-1  |     operation.database_forwards(
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/migrations/operations/special.py", line 106, in database_forwards
web-1  |     self._run_sql(schema_editor, self.sql)
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/migrations/operations/special.py", line 133, in _run_sql
web-1  |     schema_editor.execute(statement, params=None)
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/backends/postgresql/schema.py", line 45, in execute
web-1  |     return super().execute(sql, params)
web-1  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/backends/base/schema.py", line 201, in execute
web-1  |     cursor.execute(sql, params)
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/backends/utils.py", line 102, in execute
web-1  |     return super().execute(sql, params)
web-1  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/sentry_sdk/utils.py", line 1860, in runner
web-1  |     return sentry_patched_function(*args, **kwargs)
web-1  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/sentry_sdk/integrations/django/__init__.py", line 653, in execute
web-1  |     result = real_execute(self, sql, params)
web-1  |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/backends/utils.py", line 67, in execute
web-1  |     return self._execute_with_wrappers(
web-1  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/backends/utils.py", line 80, in _execute_with_wrappers
web-1  |     return executor(sql, params, many, context)
web-1  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/backends/utils.py", line 84, in _execute
web-1  |     with self.db.wrap_database_errors:
web-1  |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/utils.py", line 91, in __exit__
web-1  |     raise dj_exc_value.with_traceback(traceback) from exc_value
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/backends/utils.py", line 87, in _execute
web-1  |     return self.cursor.execute(sql)
web-1  |            ^^^^^^^^^^^^^^^^^^^^^^^^
web-1  | django.db.utils.ProgrammingError: view "auth_user" does not exist
web-1  | 
web-1  | Cache table 'external_asset_cache' already exists.
web-1  | Cache table 'durable_cache' already exists.
web-1  | Cache table 'imagekit_cache' already exists.
web-1  | System check identified some issues:
web-1  | 
web-1  | WARNINGS:
web-1  | ?: (staticfiles.W004) The directory '/src/static' in the STATICFILES_DIRS setting does not exist.
web-1  | Operations to perform:
web-1  |   Apply all migrations: admin, ai_chat, articles, auth, authentication, channels, contenttypes, data_fixtures, guardian, learning_resources, learning_resources_search, news_events, oauth2_provider, profiles, sessions, sites, social_django, testimonials, users, widgets
web-1  | Running migrations:
web-1  |   Applying users.0006_remove_auth_user_view...Traceback (most recent call last):
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/backends/utils.py", line 87, in _execute
web-1  |     return self.cursor.execute(sql)
web-1  |            ^^^^^^^^^^^^^^^^^^^^^^^^
web-1  | psycopg2.errors.UndefinedTable: view "auth_user" does not exist
web-1  | 
web-1  | 
web-1  | The above exception was the direct cause of the following exception:
web-1  | 
web-1  | Traceback (most recent call last):
web-1  |   File "/src/manage.py", line 29, in <module>
web-1  |     execute_from_command_line(sys.argv)
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line
web-1  |     utility.execute()
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/core/management/__init__.py", line 436, in execute
web-1  |     self.fetch_command(subcommand).run_from_argv(self.argv)
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/core/management/base.py", line 412, in run_from_argv
web-1  |     self.execute(*args, **cmd_options)
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/core/management/base.py", line 458, in execute
web-1  |     output = self.handle(*args, **options)
web-1  |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/core/management/base.py", line 106, in wrapper
web-1  |     res = handle_func(*args, **kwargs)
web-1  |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/core/management/commands/migrate.py", line 356, in handle
web-1  |     post_migrate_state = executor.migrate(
web-1  |                          ^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/migrations/executor.py", line 135, in migrate
web-1  |     state = self._migrate_all_forwards(
web-1  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/migrations/executor.py", line 167, in _migrate_all_forwards
web-1  |     state = self.apply_migration(
web-1  |             ^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/migrations/executor.py", line 252, in apply_migration
web-1  |     state = migration.apply(state, schema_editor)
web-1  |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/migrations/migration.py", line 132, in apply
web-1  |     operation.database_forwards(
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/migrations/operations/special.py", line 106, in database_forwards
web-1  |     self._run_sql(schema_editor, self.sql)
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/migrations/operations/special.py", line 133, in _run_sql
web-1  |     schema_editor.execute(statement, params=None)
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/backends/postgresql/schema.py", line 45, in execute
web-1  |     return super().execute(sql, params)
web-1  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/backends/base/schema.py", line 201, in execute
web-1  |     cursor.execute(sql, params)
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/backends/utils.py", line 102, in execute
web-1  |     return super().execute(sql, params)
web-1  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/sentry_sdk/utils.py", line 1860, in runner
web-1  |     return sentry_patched_function(*args, **kwargs)
web-1  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/sentry_sdk/integrations/django/__init__.py", line 653, in execute
web-1  |     result = real_execute(self, sql, params)
web-1  |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/backends/utils.py", line 67, in execute
web-1  |     return self._execute_with_wrappers(
web-1  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/backends/utils.py", line 80, in _execute_with_wrappers
web-1  |     return executor(sql, params, many, context)
web-1  |            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/backends/utils.py", line 84, in _execute
web-1  |     with self.db.wrap_database_errors:
web-1  |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/utils.py", line 91, in __exit__
web-1  |     raise dj_exc_value.with_traceback(traceback) from exc_value
web-1  |   File "/opt/venv/lib/python3.12/site-packages/django/db/backends/utils.py", line 87, in _execute
web-1  |     return self.cursor.execute(sql)
web-1  |            ^^^^^^^^^^^^^^^^^^^^^^^^
web-1  | django.db.utils.ProgrammingError: view "auth_user" does not exist

mbertrand

👍

Fix SCIM view tests

abd169b

rhysyngsun added the Needs Review An open Pull Request that is ready for review label Feb 27, 2025

gemini-code-assist bot reviewed Feb 27, 2025

View reviewed changes

mbertrand self-assigned this Feb 27, 2025

Fix user migrations for scim fields

490dcb4

rhysyngsun force-pushed the nl/fix-user-migrations branch from f3b4c96 to 490dcb4 Compare February 27, 2025 20:01

mbertrand approved these changes Feb 27, 2025

View reviewed changes

mbertrand added Waiting on author and removed Needs Review An open Pull Request that is ready for review labels Feb 27, 2025

rhysyngsun merged commit 736b810 into main Feb 27, 2025
11 checks passed

rhysyngsun deleted the nl/fix-user-migrations branch February 27, 2025 20:10

This was referenced Feb 27, 2025

Release 0.30.7 #2080

Closed

Release 0.30.7 #2085

Closed

Release 0.31.0 #2086

Closed

Release 0.30.7 #2088

Closed

Release 0.30.7 #2091

Closed

Release 0.30.7 #2092

Closed

Release 0.30.7 #2106

Closed

Release 0.30.7 #2108

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix user migrations for SCIM #2078

Fix user migrations for SCIM #2078

Uh oh!

rhysyngsun commented Feb 27, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

mbertrand commented Feb 27, 2025

Uh oh!

mbertrand left a comment

Uh oh!

Uh oh!

Uh oh!

Fix user migrations for SCIM #2078

Fix user migrations for SCIM #2078

Uh oh!

Conversation

rhysyngsun commented Feb 27, 2025

What are the relevant tickets?

Description (What does it do?)

How can this be tested?

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Changelog

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Summary of Findings

Merge Readiness

Uh oh!

mbertrand commented Feb 27, 2025

Uh oh!

mbertrand left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!