Skip to content
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

fix: recursively fetch all tags in topic area #4486

Merged
merged 1 commit into from
Jan 24, 2025
Merged

Conversation

mlbrgl
Copy link
Member

@mlbrgl mlbrgl commented Jan 24, 2025

Screenshot 2025-01-24 at 12.12.42.png

The data catalog area filters are not handling sub-areas, they only go one level deep.

We recently added sub-areas to the graph, so the real topics were "hidden" on level below.

This doesn't address:

  • type error in getTopicsForRibbons
  • other places areas might be considered a one-level deep tree structure

Note that ribbons continue to show all children (and now grandchildren tags), not just the topic tags (that is tags with a matching topic page)

Copy link
Member Author

mlbrgl commented Jan 24, 2025

This stack of pull requests is managed by Graphite. Learn more about stacking.

@mlbrgl mlbrgl changed the title fix: recursively descend into areas feat: recursively fetch all tags in topic area Jan 24, 2025
@mlbrgl mlbrgl marked this pull request as ready for review January 24, 2025 11:17
@mlbrgl mlbrgl changed the title feat: recursively fetch all tags in topic area fix: recursively fetch all tags in topic area Jan 24, 2025
@owidbot
Copy link
Contributor

owidbot commented Jan 24, 2025

Quick links (staging server):

Site Dev Site Preview Admin Wizard Docs

Login: ssh owid@staging-site-fix-datacatalog-tags

SVG tester:

Number of differences (default views): 0 ✅
Number of differences (all views): 0 ✅

Edited: 2025-01-24 11:28:03 UTC
Execution time: 1.21 seconds

Copy link
Member Author

mlbrgl commented Jan 24, 2025

@ikesau merging now to fix the most visible issue based on blackbox confirmation from @danyx23.

I'll let you take a less "quick fix" look at this post-deploy to make sure we've got all cases covered.

Copy link
Member Author

mlbrgl commented Jan 24, 2025

Merge activity

  • Jan 24, 6:31 AM EST: A user started a stack merge that includes this pull request via Graphite.
  • Jan 24, 6:31 AM EST: A user merged this pull request with Graphite.

@mlbrgl mlbrgl merged commit ec1d3e9 into master Jan 24, 2025
28 checks passed
@mlbrgl mlbrgl deleted the fix-datacatalog-tags branch January 24, 2025 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants