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

✨ add data page metadata fields to admin #3006

Conversation

Marigold
Copy link
Contributor

@Marigold Marigold commented Dec 11, 2023

Implementation of #2523.

Indicator page

  • Add all new fields, including updatePeriodDays
  • Add a list of origins for an indicator (all fields except license)
  • Add indicator source (there's always max 1)

Dataset page

  • Add origins to admin/api/datasets/X.json
  • Add updatePeriodDays
  • Add a list of origins for all indicators from a dataset
  • Add a list of dataset sources and remove confusing fields that use the first dataset source

Notes

  • I initially wanted to put field help text from our ETL docs into secondaryLabel, but eventually decided to only include link to our docs as the help would get outdated soon
  • There's still the old confusing Source on Dataset page. Perhaps we could remove it and add sources to indicator tab instead?
  • None of the fields are editable (turning editing on probably won't work for JSON fields... or maybe none of the fields)
  • JSON fields like grapherConfigETL are shown as plain text
  • Old indicators show empty data page metadata and empty origins (to shame us)
  • Let me know if I'm misusing HTML rows / cols structure

Examples:

@Marigold Marigold linked an issue Dec 11, 2023 that may be closed by this pull request
5 tasks
@Marigold Marigold force-pushed the 2523-show-new-metadata-fields-in-admin-variable-and-dataset-pages branch from 555e54f to 2e817b6 Compare December 11, 2023 12:18
@Marigold Marigold force-pushed the 2523-show-new-metadata-fields-in-admin-variable-and-dataset-pages branch from 2e817b6 to a44ccfa Compare December 11, 2023 12:27
@Marigold Marigold requested a review from danyx23 December 11, 2023 12:28
@Marigold
Copy link
Contributor Author

@danyx23 added sources to indicator and dataset admin pages.

Copy link
Contributor

@danyx23 danyx23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I think we should probably add a limit clause to the sources query for datasets. The reason is that for some extreme datasets there are hundreds or thousands of sources. Maybe limit it to 50 and add a note above or below the list?

I also removed the singular source on datasets as we no longer need it there

Copy link
Contributor

@danyx23 danyx23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should limit the number of sources we retrieve, otherwise this is good to ship!

@Marigold Marigold merged commit 46a79e7 into master Dec 19, 2023
12 of 13 checks passed
@Marigold Marigold deleted the 2523-show-new-metadata-fields-in-admin-variable-and-dataset-pages branch December 19, 2023 15:08
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.

Show new metadata fields in admin variable and dataset pages
2 participants