-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
docs: adr for data storage in course_roles djangoapp #33241
Merged
Merged
Changes from all commits
Commits
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
56 changes: 56 additions & 0 deletions
56
openedx/core/djangoapps/course_roles/docs/decisions/0002-data_storage
This file contains 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,56 @@ | ||
2. Data Storage | ||
################ | ||
|
||
Status | ||
****** | ||
|
||
**Provisional** *2023-09-13* | ||
|
||
Context | ||
******* | ||
|
||
The requirements of the course roles project indicate that equal priority be given to user understanding of the roles and flexibility of the system. It has been determined that in order to prioritize user understadning of roles and permissions it is necessary that the descriptions be localized. | ||
|
||
**Localization:** It is necessary for all text that will be displayed in the LMS to be shown in the appropriate language. The current process for translation requires that all strings displayed to users be in strings in the codebase. | ||
|
||
**Flexibility:** It is necessary that course_roles allows for the easy creation of new roles. It is considered important, but not strictly required, that each instance of the Open edX code can have its own roles without needing to fork the repo and maintain separate code. | ||
|
||
Decision | ||
******** | ||
|
||
We will be using a combination of data storage in the database and data storage structures within the code. | ||
We will store the roles, permissions, role permissions (rolepermission table), and role assignments (userrole table) in the database. | ||
We will store the role names (for default roles), permission names, and permission descriptions in a data object in the code. | ||
We will write code that pulls the role name from the database table for any role not found in the data object in the code. This applies to non-default roles that may differ from instance to instance. | ||
We will update both the code and the database table rows if a new permission is added. | ||
We will update both the code and the database table rows if a new default row is added. | ||
We will only add data in the database if a role is being added to a single Open edX instance. | ||
|
||
Consequences | ||
************ | ||
|
||
This decision will mean that any futurue default role additions or permissions changes will require changes in the code and the database. It also means that there is a chance of a default role name being listed in the UI using the name value in the database. This would occur if a role was added to the database, but the role was not added to the data structure in the code. | ||
|
||
This decision will allow for roles to be added to one instance of Open edX and not others. This can be achieved by adding the role in the database for the instance, but intentionally not adding it in the data object in the code. It is important that the role name added in the database is in the language that it should be displayed in, in the UI, because it will not be translated. | ||
|
||
This decision allows the course_roles djangoapp to utilize the same translation processes that are already in use for the codebase, with no changes. | ||
|
||
This decision allows the system to utillize database querying functionality to determine the permissions that belong to a role and by association to a user. It benefits from the database's ability to quickly return query results. | ||
|
||
Rejected Alternatives | ||
********************* | ||
|
||
* Code Based Data Objects Only - Utilize dictionaries, constants, etc to create roles and permissions | ||
* Pros: Allows for use of all Open edX defined i18n best practices | ||
* Cons: Does not allow for different roles on different systems, Slower data querying, | ||
* Database Only - Utilize database tables to store data and store translation data for the strings | ||
* Pros: Allows for different roles on different instances, Allows for easy addition of new roles | ||
* Cons: Requires custom built translation option | ||
|
||
|
||
The importance of i18n of the text displayed to users would support storing the data in the code. The importance of flexibility in creating new roles and between instances would support storing the data in a well structured set of database tables. Neither solution supports both requirements. | ||
|
||
References | ||
********** | ||
|
||
`Writing Code for Internationalization ReadTheDocs <https://edx.readthedocs.io/projects/edx-developer-guide/en/latest/internationalization/index.html>`_ |
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.
To me, this sounds like there are two sources of truth, and I think there's a simpler approach.
Namely:
pgettext
to translate them into the user's locale if a translation is available.