-
-
Notifications
You must be signed in to change notification settings - Fork 302
CommunityLibrarySubmission model #5156
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
Merged
AlexVelezLl
merged 8 commits into
learningequality:community-channels
from
Jakoma02:community-library-submission-model
Jul 8, 2025
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
97122d1
CommunityLibrarySubmission model
Jakoma02 6a3c5c4
CommunityLibrarySubmission tests
Jakoma02 fcc7dbd
Update CommunityLibrarySubmission to address PR feedback
Jakoma02 c296df1
Don't use list as default for categories
Jakoma02 dc50835
Import community library submission constants as a whole module
Jakoma02 7f17512
Move author validation to save method
Jakoma02 f0a05eb
Add channel version checks
Jakoma02 cb61cb1
Do not use translation for validation errors
Jakoma02 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
11 changes: 11 additions & 0 deletions
11
contentcuration/contentcuration/constants/community_library_submission.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| STATUS_PENDING = "PENDING" | ||
| STATUS_APPROVED = "APPROVED" | ||
| STATUS_REJECTED = "REJECTED" | ||
| STATUS_LIVE = "LIVE" | ||
|
|
||
| status_choices = ( | ||
| (STATUS_PENDING, "Pending"), | ||
| (STATUS_APPROVED, "Approved"), | ||
| (STATUS_REJECTED, "Rejected"), | ||
| (STATUS_LIVE, "Live"), | ||
| ) |
91 changes: 91 additions & 0 deletions
91
contentcuration/contentcuration/migrations/0154_community_library_submission.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| # Generated by Django 3.2.24 on 2025-07-03 17:06 | ||
| import django.db.models.deletion | ||
| from django.conf import settings | ||
| from django.db import migrations | ||
| from django.db import models | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
|
|
||
| dependencies = [ | ||
| ("contentcuration", "0153_alter_recommendationsevent_time_hidden"), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.CreateModel( | ||
| name="Country", | ||
| fields=[ | ||
| ( | ||
| "code", | ||
| models.CharField( | ||
| help_text="alpha-2 country code", | ||
| max_length=2, | ||
| primary_key=True, | ||
| serialize=False, | ||
| ), | ||
| ), | ||
| ("name", models.CharField(max_length=100, unique=True)), | ||
| ], | ||
| ), | ||
| migrations.CreateModel( | ||
| name="CommunityLibrarySubmission", | ||
| fields=[ | ||
| ( | ||
| "id", | ||
| models.AutoField( | ||
| auto_created=True, | ||
| primary_key=True, | ||
| serialize=False, | ||
| verbose_name="ID", | ||
| ), | ||
| ), | ||
| ("description", models.TextField(blank=True)), | ||
| ("channel_version", models.PositiveIntegerField()), | ||
| ("categories", models.JSONField(blank=True, null=True)), | ||
| ("date_created", models.DateTimeField(auto_now_add=True)), | ||
| ( | ||
| "status", | ||
| models.CharField( | ||
| choices=[ | ||
| ("PENDING", "Pending"), | ||
| ("APPROVED", "Approved"), | ||
| ("REJECTED", "Rejected"), | ||
| ("LIVE", "Live"), | ||
| ], | ||
| default="PENDING", | ||
| max_length=20, | ||
| ), | ||
| ), | ||
| ( | ||
| "author", | ||
| models.ForeignKey( | ||
| on_delete=django.db.models.deletion.CASCADE, | ||
| related_name="community_library_submissions", | ||
| to=settings.AUTH_USER_MODEL, | ||
| ), | ||
| ), | ||
| ( | ||
| "channel", | ||
| models.ForeignKey( | ||
| on_delete=django.db.models.deletion.CASCADE, | ||
| related_name="community_library_submissions", | ||
| to="contentcuration.channel", | ||
| ), | ||
| ), | ||
| ( | ||
| "countries", | ||
| models.ManyToManyField( | ||
| related_name="community_library_submissions", | ||
| to="contentcuration.Country", | ||
| ), | ||
| ), | ||
| ], | ||
| ), | ||
| migrations.AddConstraint( | ||
| model_name="communitylibrarysubmission", | ||
| constraint=models.UniqueConstraint( | ||
| fields=("channel", "channel_version"), | ||
| name="unique_channel_with_channel_version", | ||
| ), | ||
| ), | ||
| ] |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
We usually do these validations at api level, but can imagine the benefits of doing this at model level. This doesn't seem to be a common pattern in our codebase thought, would like to hear @rtibbles thoughts, as we may also need to do the check of the version being
version > 0 and version <= channel.versionUh oh!
There was an error while loading. Please reload this page.
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.
When we have done these kinds of validations on the model itself, we have usually done it in the
savemethod rather than the clean method - for precisely the reasons noted in the docstring - clean is not called on every save, but only when using a Django model form (which we rarely use).If we want to ensure that this is being used every time we save a model, we should put it in the save method instead, otherwise, I think it would sit better in the API endpoint.
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.
Okay, if you agree, I will move the logic to the
savemethod.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.
Yes, lets move this logic to the save method, along with the version check. Thanks!
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.
Added in 7f17512 and f0a05eb.
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.
Thank youu! <3 Just one little thing before we merge (I promise this is the last one 😅):
Could you remove the translate function from the error messages? We use to return these validation error messages without translating them e.g. here.
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.
I removed translation in cb61cb1.
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.
Thanks!