fix: Fix bugs in the "get tags" REST API, make it more consistent [FAL-3530] [FC-0036] #119

bradenmacdonald · 2023-11-17T19:46:17Z

This change fixes some bugs and inconsistencies in the "get tags" REST API. Hopefully it will be much easier to understand and use now.

Bugs:

Before, the pagination and tree depth behavior of the function would be different depending on whether a search term was used or not.
Before, by default, the pagination format and depth of the results would depend on the overall size of the taxonomy, which could produce unexpected results for API consumers who aren't aware of this.
Before, the pagination would sometimes put a child on a different page than its parent, which was not incorrect but could be confusing.
When using a search term and loading sub tags using "sub_tags_url", the search term filter would be lost and all children would be returned, even if they didn't match the search.

New behavior:

Only a single layer of results is returned by default, and it's paginated by the page size (which you can customize using ?page_size).

Specifying a search term will filter the results as before, but no longer has any impact on the pagination or depth of the results. By default, only a single layer of results is returned. e.g. if you search this taxonomy

openedx-learning/tests/openedx_tagging/core/fixtures/tagging.yaml

Lines 1 to 21 in 2a983c0

    
           # - Bacteria 
        
           #    |- Archaebacteria 
        
           #    |- Eubacteria 
        
           # - Archaea 
        
           #    |- DPANN 
        
           #    |- Euryarchaeida 
        
           #    |- Proteoarchaeota 
        
           # - Eukaryota 
        
           #    |- Animalia 
        
           #    |   |- Arthropoda 
        
           #    |   |- Chordata 
        
           #    |   |   |- Mammalia 
        
           #    |   |- Cnidaria 
        
           #    |   |- Ctenophora 
        
           #    |   |- Gastrotrich 
        
           #    |   |- Placozoa 
        
           #    |   |- Porifera 
        
           #    |- Fungi 
        
           #    |- Monera 
        
           #    |- Plantae 
        
           #    |- Protista

for "Arthropoda", you'll only get "Eukaryota" as the result, because it's the root tag that has a grandchild that matches. You can then recursively follow the sub_tags_url on "Eukaryota" and you'll get "Animalia" and then follow sub_tags_url again you'll get "Arthropoda". However, there's a more efficient way!

You can now opt-in to a "smart" behavior: using ?full_depth_threshold=1000, any response that would return fewer than 1,000 tags will return a single giant page of results with all the tags in tree order, up to the maximum supported search depth (3). Unlike before, this now works with any type of query, whether you're searching or not, loading root tags or some deeper tags, or anything else. Also unlike before, this is now based on the number of results that are returned, not the size of the taxonomy.

The ?root_only parameter has been removed because it's now on by default. If you want the children included in the result (if possible), you just opt-in to that using ?full_depth_threshold.

There are no changes at all to the python API.

Also, this change seems to work fine with the current implementation of the tagging drawer (i.e. despite the major changes, it's mostly backwards-compatible).

openedx-webhooks · 2023-11-17T19:46:23Z

Thanks for the pull request, @bradenmacdonald! 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:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

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.

bradenmacdonald · 2023-11-17T19:56:54Z

@ChrisChV @yusuf-musleh Here's my proposed fix for the inconsistency when searching in the "list tags" API. Can you let me know your thoughts?

bradenmacdonald · 2023-11-17T20:17:10Z

BTW @yusuf-musleh what I would recommend for the frontend is using a few different combinations of parameters like ?page_size=5&full_depth_threshold=0, ?full_depth_threshold=20, and ?full_depth_threshold=10000 during development to make sure that everything is working correctly: populating the children if they're included in the result, "load more" for root tags if there's more than one page, "load more" for child tags if the children are paginated, and loading sub-tags as needed.

yusuf-musleh · 2023-11-21T11:07:33Z

@ChrisChV @yusuf-musleh Here's my proposed fix for the inconsistency when searching in the "list tags" API. Can you let me know your thoughts?

@bradenmacdonald Thanks for simplifying the endpoint and keeping it more consistent. I agree, its a lot more predictable and less confusing when they are more deliberate, giving the API consumer the option to pick how they want their data to appear.

I set this up locally and tried it out with the tags drawer, and it fixed things up quite well. However I noticed some inconsistencies with the search results, namely when searching without the full_depth_threshold param.

It seems like results only show up when the search term exists at the top level. For example, when searching in the Light Cast taxonomy, that looks like so:

http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/

{
  "next":null,
  "previous":null,
  "count":7,
  "num_pages":1,
  "current_page":1,
  "start":0,
  "results":[
    {
      "value":"Abilities",
      "external_id":"A",
      "child_count":4,
      "depth":0,
      "parent_value":null,
      "_id":626250,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Abilities"
    },
    {
      "value":"Interests",
      "external_id":"C",
      "child_count":1,
      "depth":0,
      "parent_value":null,
      "_id":626373,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Interests"
    },
    {
      "value":"Knowledge",
      "external_id":"G",
      "child_count":11,
      "depth":0,
      "parent_value":null,
      "_id":626442,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Knowledge"
    },
    {
      "value":"Personal Attributes",
      "external_id":"B",
      "child_count":5,
      "depth":0,
      "parent_value":null,
      "_id":626326,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Personal+Attributes"
    },
    {
      "value":"Skills",
      "external_id":"F",
      "child_count":5,
      "depth":0,
      "parent_value":null,
      "_id":626382,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Skills"
    },
    {
      "value":"Work Activities",
      "external_id":"K",
      "child_count":4,
      "depth":0,
      "parent_value":null,
      "_id":626603,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Work+Activities"
    },
    {
      "value":"Work Context",
      "external_id":"J",
      "child_count":4,
      "depth":0,
      "parent_value":null,
      "_id":626511,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Work+Context"
    }
  ]
}

If we take a look at "Abilities", it looks like so:
http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Abilities

{
  "next":null,
  "previous":null,
  "count":4,
  "num_pages":1,
  "current_page":1,
  "start":0,
  "results":[
    {
      "value":"Cognitive Abilities",
      "external_id":"A01",
      "child_count":6,
      "depth":1,
      "parent_value":"Abilities",
      "_id":626251,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Cognitive+Abilities"
    },
    {
      "value":"Physical Abilities",
      "external_id":"A02",
      "child_count":3,
      "depth":1,
      "parent_value":"Abilities",
      "_id":626279,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Physical+Abilities"
    },
    {
      "value":"Psychomotor Abilities",
      "external_id":"A03",
      "child_count":3,
      "depth":1,
      "parent_value":"Abilities",
      "_id":626293,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Psychomotor+Abilities"
    },
    {
      "value":"Sensory Abilities",
      "external_id":"A04",
      "child_count":3,
      "depth":1,
      "parent_value":"Abilities",
      "_id":626307,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Sensory+Abilities"
    }
  ]
}

If I search at the top level for "Sensory", I get the following:
http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?search_term=sensory

{
  "next":null,
  "previous":null,
  "count":0,
  "num_pages":1,
  "current_page":1,
  "start":0,
  "results":[
    
  ]
}

However if we search for "sensory" within "Abilities" we get the results:
http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Abilities&search_term=sensory

{
  "next":null,
  "previous":null,
  "count":1,
  "num_pages":1,
  "current_page":1,
  "start":0,
  "results":[
    {
      "value":"Sensory Abilities",
      "external_id":"A04",
      "child_count":3,
      "depth":1,
      "parent_value":"Abilities",
      "_id":626307,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Sensory+Abilities&search_term=sensory"
    }
  ]
}

This is consistent all levels. However if I pass in the full_depth_threshold query, I get the expected results:

http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?search_term=sensory&full_depth_threshold=1000

{
  "next":null,
  "previous":null,
  "count":3,
  "num_pages":1,
  "current_page":1,
  "start":0,
  "results":[
    {
      "value":"Abilities",
      "external_id":"A",
      "child_count":4,
      "depth":0,
      "parent_value":null,
      "_id":626250,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Abilities&full_depth_threshold=1000&search_term=sensory"
    },
    {
      "value":"Sensory Abilities",
      "external_id":"A04",
      "child_count":3,
      "depth":1,
      "parent_value":"Abilities",
      "_id":626307,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Sensory+Abilities&full_depth_threshold=1000&search_term=sensory"
    },
    {
      "value":"Other Sensory Abilities",
      "external_id":"A04c",
      "child_count":4,
      "depth":2,
      "parent_value":"Sensory Abilities",
      "_id":626321,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Other+Sensory+Abilities&full_depth_threshold=1000&search_term=sensory"
    }
  ]
}

Here is another example if I search for "Other" which exists 2 levels deep with and without full_depth_threshold.

Without full_depth_threshold

http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?search_term=other

{
  "next":null,
  "previous":null,
  "count":0,
  "num_pages":1,
  "current_page":1,
  "start":0,
  "results":[
    
  ]
}

With full_depth_threshold

http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?search_term=other&full_depth_threshold=1000

{
  "next":null,
  "previous":null,
  "count":5,
  "num_pages":1,
  "current_page":1,
  "start":0,
  "results":[
    {
      "value":"Abilities",
      "external_id":"A",
      "child_count":4,
      "depth":0,
      "parent_value":null,
      "_id":626250,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Abilities&full_depth_threshold=1000&search_term=other"
    },
    {
      "value":"Cognitive Abilities",
      "external_id":"A01",
      "child_count":6,
      "depth":1,
      "parent_value":"Abilities",
      "_id":626251,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Cognitive+Abilities&full_depth_threshold=1000&search_term=other"
    },
    {
      "value":"Other Cognitive Abilities",
      "external_id":"A01f",
      "child_count":3,
      "depth":2,
      "parent_value":"Cognitive Abilities",
      "_id":626275,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Other+Cognitive+Abilities&full_depth_threshold=1000&search_term=other"
    },
    {
      "value":"Sensory Abilities",
      "external_id":"A04",
      "child_count":3,
      "depth":1,
      "parent_value":"Abilities",
      "_id":626307,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Sensory+Abilities&full_depth_threshold=1000&search_term=other"
    },
    {
      "value":"Other Sensory Abilities",
      "external_id":"A04c",
      "child_count":4,
      "depth":2,
      "parent_value":"Sensory Abilities",
      "_id":626321,
      "sub_tags_url":"http://localhost:18010/api/content_tagging/v1/taxonomies/14/tags/?parent_tag=Other+Sensory+Abilities&full_depth_threshold=1000&search_term=other"
    }
  ]
}

If I understood this correctly, when the result is not in the top level but within its subtags (and the full_depth_threshold is not included), its still should return those top level tags with subtag urls containing the search term so you can expand it further to view the search results.

yusuf-musleh · 2023-11-21T11:09:29Z

BTW @yusuf-musleh what I would recommend for the frontend is using a few different combinations of parameters like ?page_size=5&full_depth_threshold=0, ?full_depth_threshold=20, and ?full_depth_threshold=10000 during development to make sure that everything is working correctly: populating the children if they're included in the result, "load more" for root tags if there's more than one page, "load more" for child tags if the children are paginated, and loading sub-tags as needed.

Will do! I still haven't implemented the "Load more" functionality/button, I have it on my todo list. This would definitely help with testing it out.

ChrisChV · 2023-11-21T15:31:20Z

Here's my proposed fix for the inconsistency when searching in the "list tags" API. Can you let me know your thoughts?

@bradenmacdonald I took a quick look, and I like your proposal. Could you updated the ADR with this new?

…match

bradenmacdonald · 2023-11-21T19:00:15Z

@yusuf-musleh Good catch. I fixed the bug with 8c4c0dd and included a test case for it.

@ChrisChV Sure thing. ADR is updated.

bradenmacdonald · 2023-11-21T19:01:23Z

@pomegranited If you have time, would you be able to review this as CC?

I can include a version bump when I merge.

pomegranited

👍 once nits are addressed.

I tested this on my devstack using the latest edx-platform and Course Authoring MFE, with data generated from the taxonomy sample data.
I read through the code
~~I checked for accessibility issues~~ N/A
Updates documentation
~~User-facing strings are extracted for translation~~ N/A

docs/decisions/0014-single-taxonomy-view-api.rst

openedx_tagging/core/tagging/rest_api/v1/views.py

openedx-webhooks · 2023-11-23T17:49:18Z

@bradenmacdonald 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

fix: Fix bugs in the "get tags" REST API, make it more consistent

0116625

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Nov 17, 2023

bradenmacdonald mentioned this pull request Nov 17, 2023

feat: display all child tags in the "bare bones" taxonomy detail page [FAL-3529] [FC-0036] open-craft/frontend-app-authoring#10

Closed

bradenmacdonald added the FC Relates to an Axim Funded Contribution project label Nov 20, 2023

bradenmacdonald mentioned this pull request Nov 20, 2023

feat: display all child tags in the "bare bones" taxonomy detail page [FAL-3529] [FC-0036] openedx/frontend-app-authoring#703

Merged

bradenmacdonald added 2 commits November 21, 2023 10:26

fix: shallow search was not returning parent tags with children that …

8c4c0dd

…match

docs: update the ADR that describes the "get tags" endpoint

3adc19d

bradenmacdonald requested a review from pomegranited November 21, 2023 19:00

pomegranited approved these changes Nov 22, 2023

View reviewed changes

docs/decisions/0014-single-taxonomy-view-api.rst Outdated Show resolved Hide resolved

docs/decisions/0014-single-taxonomy-view-api.rst Outdated Show resolved Hide resolved

openedx_tagging/core/tagging/rest_api/v1/views.py Outdated Show resolved Hide resolved

yusuf-musleh reviewed Nov 22, 2023

View reviewed changes

openedx_tagging/core/tagging/rest_api/v1/views.py Outdated Show resolved Hide resolved

fix: don't throw an error when retrieving tags below a leaf tag

029a17b

bradenmacdonald force-pushed the fix-get-children-tags branch from 5219d1c to 3452e8e Compare November 22, 2023 21:31

bradenmacdonald added 2 commits November 22, 2023 13:35

feat: return the number of matching children only with ?search_term=...

333911f

fix: address feedback from review

1a33e47

bradenmacdonald force-pushed the fix-get-children-tags branch from 3452e8e to 1a33e47 Compare November 22, 2023 21:35

bradenmacdonald added 2 commits November 22, 2023 13:36

chore: update with latest main branch

5cc794b

chore: Version bump

564470e

bradenmacdonald merged commit b98f091 into openedx:main Nov 23, 2023

bradenmacdonald deleted the fix-get-children-tags branch November 23, 2023 17:49

ormsbee mentioned this pull request Mar 24, 2025

add vscode pytest settings open-craft/openedx-learning#19

Closed

ormsbee mentioned this pull request Sep 4, 2025

version bump 0.28.0 open-craft/openedx-learning#20

Closed

	# - Bacteria
	# \|- Archaebacteria
	# \|- Eubacteria
	# - Archaea
	# \|- DPANN
	# \|- Euryarchaeida
	# \|- Proteoarchaeota
	# - Eukaryota
	# \|- Animalia
	# \| \|- Arthropoda
	# \| \|- Chordata
	# \| \| \|- Mammalia
	# \| \|- Cnidaria
	# \| \|- Ctenophora
	# \| \|- Gastrotrich
	# \| \|- Placozoa
	# \| \|- Porifera
	# \|- Fungi
	# \|- Monera
	# \|- Plantae
	# \|- Protista

fix: Fix bugs in the "get tags" REST API, make it more consistent [FAL-3530] [FC-0036] #119

fix: Fix bugs in the "get tags" REST API, make it more consistent [FAL-3530] [FC-0036] #119

Uh oh!

Conversation

bradenmacdonald commented Nov 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openedx-webhooks commented Nov 17, 2023

Uh oh!

bradenmacdonald commented Nov 17, 2023

Uh oh!

bradenmacdonald commented Nov 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yusuf-musleh commented Nov 21, 2023

Uh oh!

yusuf-musleh commented Nov 21, 2023

Uh oh!

ChrisChV commented Nov 21, 2023

Uh oh!

bradenmacdonald commented Nov 21, 2023

Uh oh!

bradenmacdonald commented Nov 21, 2023

Uh oh!

pomegranited left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

openedx-webhooks commented Nov 23, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bradenmacdonald commented Nov 17, 2023 •

edited

Loading

bradenmacdonald commented Nov 17, 2023 •

edited

Loading