docs: add ADR 0022 assessment criteria model#474
docs: add ADR 0022 assessment criteria model#474mgwozdz-unicon wants to merge 3 commits intoopenedx:mainfrom
Conversation
|
Thanks for the pull request, @mgwozdz-unicon! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
bradenmacdonald
left a comment
There was a problem hiding this comment.
Would you mind providing an end-to-end example of the data in each of these models?
I'm trying to understand the need for both ObjectTag and oel_assessment_criteria.
e.g. If the competency is "Writing Poetry" and "Unit 7: Submit a Poem" has the ObjectTag [Unit7->Writing Poetry], then we need oel_assessment_criteria to attach additional data to that specific object in that specific course?
e.g. The criteria group would be "To achieve the 'Writing Poetry' Competency in course X, you have to: (Criteria 1: achieve a grade of 75% or better on [Unit7->Writing Poetry]) AND (Criteria 2: achieve a grade of 85% or better on [Unit9->Remixing Poetry])"
Do authors really want to specify the rules at such a granular level (separately and independently for each activity of each competency of each course)? I would have assumed that they just want a few high-level rules like "Earning a grade of 75% or better for all content in the course tagged with both a competency and the 'CountsForCompetency' tag will earn that competency".
| Decision | ||
| -------- | ||
|
|
||
| 1. Update `oel_tagging_taxonomy` to have a new column for `taxonomy_type` where the value could be “Competency” or “Tag”. |
There was a problem hiding this comment.
What would the purpose of this taxonomy_type field be, and what would it affect?
We actually have two taxonomy types already, free-text and static, controlled via the allow_free_text field. So it may make sense to remove that field have three taxonomy_types: "Regular", "Free text", and "Competency"
- Free text taxonomy -> no hierarchy allowed, no predefined tags, authors can "tag as you go"
- Regular taxonomy -> hierarchical tags, tags must be predefined by editing/importing the taxonomy before content can be tagged
- Competency -> ????
There was a problem hiding this comment.
This will be very different from the idea of allow_free_text so I don't think it should be combined with that. We do want to make sure that if people want to add other taxonomy types in the future, they can, but I'm going to update the ADR with the following detail to explain this further:
A taxonomy with a taxonomy_type of "Competency":
- is able to be displayed in the UI with the competency criteria association view.
- will be able to be displayed in the UI with the competency progress tracking view(s).
- has constraints on its associated content objects to only be those supported for progress tracking
- has contrainsts on its associated content objects to only include ones that could logically be used to demonstrate mastery of the competency (for example, it would not make sense to have a competency associated to both a course and a subsection assignment within that course because then there's no way to know if it's the final course grade or just that one assignment that needs to be tracked for mastery).
In contrast, a taxonomy with a taxonomy_type of "Tag":
- is only displayed in the UI with the existing Taxonomy view, not able to be displayed in the UI with the competency criteria association view.
- is not able to be displayed in the UI with the competency progress tracking view(s).
- has no constraints on its associated content objects, and can be associated with any course content object.
There was a problem hiding this comment.
OK, that's very helpful.
I agree it's a different thing: allow_free_text affects the content/structure of the taxonomy itself, but this "Competency" type still sounds exactly the same as a "regular" taxonomy in terms of its content/structure; it's just the use case that's different - same sort of taxonomy, but using it in a more structured way (only on specific objects) and enabling new UI views for its usage.
In that case, I'm wondering if we should call it use_for_competency=True or usage_type=Competency, to clarify that the actual structure/behavior of the taxonomy on the taxonomy editor side is the same.
Can a "Competency" taxonomy still be viewed in the existing taxonomy view, if desired?
There was a problem hiding this comment.
What about making a simple CompetencyTaxonomy table, with a primary 1-1 key to the Taxonomy table? Essentially, rows in CompetencyTaxonomy would "mark" the Taxonomy as suitable for use as a competency. The openedx_tagging app wouldn't need to know anything about CBE. Furthermore, other CBE-related tables could ForeignKey to CompetencyTaxonomy instead of Taxonomy, ensuring on a DB level that they are referencing a suitable taxonomy.
This is similar to how we indicate that a Container is a Sections, Subsections, or Unit, without making the containers applet aware of the sections, subsections, or units applets.
There was a problem hiding this comment.
Can a "Competency" taxonomy still be viewed in the existing taxonomy view, if desired?
Yes, and I updated the ADR to clarify this.
What about making a simple CompetencyTaxonomy table, with a primary 1-1 key to the Taxonomy table?
I think this makes sense. I've updated the ADR to use this instead of the new field.
I go through an example in my Technical Design Document here: https://openedx.atlassian.net/wiki/spaces/OEPM/pages/5604573185/Competency+Assessment+Criteria+Technical+Design+Document#Criteria-Aggregation-and-Logical-Grouping
Yes and no. Authors will need to be able to indicate assessment criteria (which we are renaming to "competency criteria") for each individual subsection assignment and in the future also for each individual rubric criteria. However, you are correct that for most folks, they will want to define a default rule at their organization, taxonomy, or course level that applies for all their competency criteria associations. To address this, I've now added a new I also considered the possibility of combining the |
- Renamed "Assessment Criteria" to "Competency Criteria" per team decision to reduce confusion regarding this term - Added details about the purpose of `taxonomy_type` - Added `oel_competency_rule_profile` table for scoped default competency rules - Removed duplicative columns from `oel_competency_criteria` table, added necessary indexes, and updated it to work with the profile table. - Added example of how the model would be used - Added Rejected Alternative for combining `oel_competency_criteria` and `oel_tagging_objecttag` tables - Updated diagram images accordingly
- Switch to Competency Taxonomy table to indicate taxonomy type instead of new field in Taxonomy table - Indicate that Competency Taxonomies can be displayed in the existing Taxonomy UI screens - Updated/removed unnecessary images
8cf124f to
4c6f809
Compare
ormsbee
left a comment
There was a problem hiding this comment.
Lots of good stuff here—these are mostly just questions on my part. Thank you!
| 1. `id`: unique primary key | ||
| 2. `parent_id`: The `oel_competency_criteria_group.id` of the group that is the parent to this one. | ||
| 3. `oel_tagging_tag_id`: The `oel_tagging_tag.id` of the tag that represents the competency that is mastered when the competency achievement criteria in this group are demonstrated. | ||
| 4. `course_id`: The nullable `course_id` to which all of the child competency achievement criteria's associated objects belong. |
There was a problem hiding this comment.
This can now be a foreign key to openedx_catalog's CourseRun
| Decision | ||
| -------- | ||
|
|
||
| 1. Add a new database table for `oel_competency_taxonomy` to track which Taxonomies are to be used as "Competency" taxnomies as opposed to as a generic tag. |
There was a problem hiding this comment.
FWIW, the level of detail you give here in terms of the schema is fine, but it's more precise than is really required at the ADR level. We do have some that are this specific, but they are often more general and talk about it at the level of what the top level model entities are and their relationship (1:1, 1:M, M:M) to each other—since more granular details often change during development.
| Example: A root group uses "OR" with two child groups. | ||
|
|
||
| - Child group A (`ordering=1`) requires "AND" across Assignment 1 and Assignment 2. | ||
| - Child group B (`ordering=2`) requires "AND" across Final Exam and viewing prerequisites. | ||
| - If group A evaluates to true, group B is not evaluated. |
There was a problem hiding this comment.
Not a request for this ADR, but just as a side note: If it is simple to display the rules in a more readable form in the Django admin, it would be very helpful. It may be cumbersome for folks who are not familiar with the models to try to grok what's going on just from looking at the database tables.
|
|
||
| This new database table will have the following columns: | ||
| 1. `id`: unique primary key | ||
| 2. `oel_tagging_taxonomy_id`: Foreign key to `oel_tagging_taxonomy.id` to identify which taxonomy is being designated as a competency taxonomy. |
There was a problem hiding this comment.
Would a CompetencyTaxonomy be a subclass of Taxonomy, i.e. use multi-table inheritance?
| 1. `id`: unique primary key | ||
| 2. `oel_tagging_taxonomy_id`: Foreign key to `oel_tagging_taxonomy.id` to identify which taxonomy is being designated as a competency taxonomy. | ||
|
|
||
| 2. Add new database table for `oel_competency_criteria_group` with these columns: |
There was a problem hiding this comment.
One thing I find myself doing in this ADR is constantly scrolling up to the top paragraph to make sure that I understand what the concept of a CompetencyCriteriaGroup is, that "competency criteria" is not different from "competency achievement criteria", etc. (Please feel free to use CamelCasedNames to be really specific about concept names.)
Suggestion: Instead of leading these sections with the table name, please try leading with the name of the concept it represents, a plain description of what it maps to (like from your first paragraph), and how it relates to other concepts. I'm interested in things like:
- Can a criteria group be empty, and if so, what is the intended behavior?
- Can a criteria group be composed of things of different depths, e.g. (section1 > 75% and section2 > 75% and (section3 > 75% or section4 > 75%))?
- Realistically speaking, how big do we expect these things to be able to grow to?
The ADR doesn't need to have definite answers for these. I just give them as examples of things that I think help to demonstrate the boundaries around these concepts.
|
|
||
| 1. `Grade`: `{"op": "gte", "value": 75, "scale": "percent"}` | ||
|
|
||
| 5. Add indexes for common lookups. |
There was a problem hiding this comment.
This is fine to keep in, but I don't believe this level of detail is necessary at the ADR level.
| 1. `id`: unique primary key | ||
| 2. `competency_criteria_id`: Foreign key pointing to competency achievement criteria id | ||
| 3. `user_id`: Foreign key pointing to user_id (presumably the learner's id, although it appears that it is possible for staff to get grades as well) in `auth_user` table | ||
| 4. `status`: “Demonstrated”, “AttemptedNotDemonstrated”, “PartiallyAttempted” |
There was a problem hiding this comment.
I'm assuming this is going to be an enumeration of some sort. I just want to caution that we'll want this field to be very small, and not a varchar(100) or something like that, especially since it'll be part of an index somewhere.
| 2. `competency_criteria_id`: Foreign key pointing to competency achievement criteria id | ||
| 3. `user_id`: Foreign key pointing to user_id (presumably the learner's id, although it appears that it is possible for staff to get grades as well) in `auth_user` table | ||
| 4. `status`: “Demonstrated”, “AttemptedNotDemonstrated”, “PartiallyAttempted” | ||
| 5. `timestamp`: The timestamp at which the student's competency achievement criteria status was set. |
There was a problem hiding this comment.
By convention, we use the field name created for this.
| 1. `id`: unique primary key | ||
| 2. `competency_criteria_group_id`: Foreign key pointing to competency achievement criteria group id | ||
| 3. `user_id`: Foreign key pointing to user_id (presumably the learner's id, although it appears that it is possible for staff to get grades as well) in `auth_user` table | ||
| 4. `status`: “Demonstrated”, “AttemptedNotDemonstrated”, “PartiallyAttempted” |
There was a problem hiding this comment.
If these are going to essentially be the same fields as student_assessmentcriteriastatus, it makes me wonder if there should be just one table that student status joins against. So in other words, if it's possible to make it so that there's just one recursive table that represents the structure of the evaluation tree, and a criteria could add its supplemental information in a 1:1 relationship to nodes in that tree, instead of that evaluation tree being conceptually a mix of two different models.
|
|
||
| 1. `id`: unique primary key | ||
| 2. `parent_id`: The `oel_competency_criteria_group.id` of the group that is the parent to this one. | ||
| 3. `oel_tagging_tag_id`: The `oel_tagging_tag.id` of the tag that represents the competency that is mastered when the competency achievement criteria in this group are demonstrated. |
There was a problem hiding this comment.
If someone tries to delete this tag, do we allow it to cascade down and delete the associated competency groups, or do we prevent the deletion with an error?
No description provided.