-
Notifications
You must be signed in to change notification settings - Fork 2
feat: replace UUID with LearningPathKey #13
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
Conversation
27ce2c3 to
41b19ce
Compare
f20c2ef to
c6587b6
Compare
| LearningPath = apps.get_model("learning_paths", "LearningPath") | ||
|
|
||
| for learning_path in LearningPath.objects.all(): | ||
| key_str = f"path-v1:test+test+test+{learning_path.uuid}" |
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.
This plugin has not been deployed anywhere yet, so this function will only affect dev environments.
learning_paths/keys.py
Outdated
|
|
||
| __slots__ = KEY_FIELDS | ||
|
|
||
| _learing_path_key_regex = re.compile(rf"^{LEARNING_PATH_PATTERN}$") |
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.
Nit: Missing n in learning.
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.
pkulkark
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.
@Agrendalath The changes look good but when I try to run it on my machine, I get this error during migration:
Applying learning_paths.0007_replace_uuid_with_learningpathkey...Traceback (most recent call last):
File "/openedx/venv/lib/python3.11/site-packages/django/db/backends/utils.py", line 89, in _execute
return self.cursor.execute(sql, params)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/openedx/venv/lib/python3.11/site-packages/django/db/backends/mysql/base.py", line 75, in execute
return self.cursor.execute(query, args)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/openedx/venv/lib/python3.11/site-packages/MySQLdb/cursors.py", line 179, in execute
res = self._query(mogrified_query)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/openedx/venv/lib/python3.11/site-packages/MySQLdb/cursors.py", line 330, in _query
db.query(q)
File "/openedx/venv/lib/python3.11/site-packages/MySQLdb/connections.py", line 261, in query
_mysql.connection.query(self, query)
MySQLdb.IntegrityError: (1062, "Duplicate entry '' for key 'learning_paths_learningpath.key'")
pkulkark
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.
@Agrendalath LGTM 👍
- I tested this: Verified existing APIs and learning paths are accessible with the new key.
- I read through the code
-
Includes documentationN/A
bd3e216 to
f9f2264
Compare
This replaces the identifiers of the Learning Paths. Instead of the UUIDs, they will be identified by LearningPathKeys.
Testing instructions
Private-ref: BB-9241