Ensure the correct Taxonomy subclass is used #60
Conversation
* Adds dependency on django-model-utils (which is already a dependency of edx-platform) * Updates api.get_taxonomies and adds api.get_taxonomy, which return the correct subclass of Taxonomy * Adds a simple SystemTaxonomy base class to verify ^ * Updates tests to ensure correct classes are used, and no extra queries are required for this functionality
|
Thanks for the pull request, @pomegranited! Please note that it may take us up to several weeks or months to complete a review and merge your PR. Feel free to add as much of the following information to the ticket as you can:
All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here. Please let us know once your PR is ready for our review and all tests are green. |
openedx-webhooks
added
the
open-source-contribution
| # FIXME: | ||
| # * cache to prevent re-fetching the taxonomy | ||
| # * discourage people from using self.taxonomy | ||
| taxonomy = ( | ||
| Taxonomy.objects.filter(id=self.taxonomy_id).select_subclasses().first() | ||
| ) |
There was a problem hiding this comment.
TODO What's the best way to handle this? We can cache the fetched taxonomy subclass using a getter property, but we'll need to rename the taxonomy field to _taxonomy.
so we can ensure that the ObjectTag.taxonomy returned is of the correct subclass.
|
Closed in favor of #62 |
|
@pomegranited Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future. |
3 participants
Description
Ensures that we always use the correct Taxonomy model subclass whenever returning or using Taxonomy instances.
Supporting information
Proposed way to address openedx/edx-platform#32518 (comment)
Testing instructions
Manual testing is possible but not very useful, since we don't yet have views for these models and APIs.
But you can check that the added tests cover the expected polymorphism behavior supported by this change, and that tests are still covering 100% of the
openedx_taggingmodule.Other information
Here we add a basic SystemTaxonomy class to help validate this change, but more functionality for system taxonomies will be added by a subsequent PR.
Author to do before merge: