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

Validate slug uniqueness also in draft charts #4470

Open
lucasrodes opened this issue Jan 20, 2025 · 1 comment
Open

Validate slug uniqueness also in draft charts #4470

lucasrodes opened this issue Jan 20, 2025 · 1 comment
Labels
admin Issues that need to be solved in the grapher admin (mostly) feature priority 3 - nice to have soonish and small Do it some time soon like next cooldown viz

Comments

@lucasrodes
Copy link
Member

lucasrodes commented Jan 20, 2025

Core problem

Draft charts don't get their slug validated to ensure it is unique.

Context

When publishing a chart, we need to ensure that its slug is unique, or Grapher will complain when attempting to publish the chart (see image below)

Image

However, if we save the chart as a draft, Grapher won't complain. This may seem irrelevant, but it can cause issues when working with staging servers & chart diff.

When working on a data project with Saloni, we have a workflow like:

  • I work on the data & import it into a staging server.
  • I share the server details with the author.
  • The author (Saloni) creates some charts with the new data and saves them as drafts.
    • Why drafts? The charts might not be finished and are just the chart's first version.
    • But if they are in a staging server, why bother? Because once we merge the PR and the charts are synced to PROD, we want to keep them as drafts.

Considering this, if we use a slug that's already in use, chart-diff will show the error:

Image

While this is caught in chart-diff (and shouldn't go unnoticed), it would be nicer if we could catch it earlier. E.g. when the author saves the draft in Admin.

Proposed solution

Apply the same validation logic to draft charts. This means running the same checks when clicking "Save draft" than when clicking on "Publish" or "Update chart".

@marcelgerber marcelgerber added the admin Issues that need to be solved in the grapher admin (mostly) label Jan 22, 2025
@marcelgerber
Copy link
Member

What would be nice here is if a slug can be null for draft charts, and only needs to be present for when a chart is published.

But otherwise, we agree with Lucas here, and it would also bring some sanity into our own lives.

@marcelgerber marcelgerber added priority 3 - nice to have soonish and small Do it some time soon like next cooldown and removed needs triage labels Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
admin Issues that need to be solved in the grapher admin (mostly) feature priority 3 - nice to have soonish and small Do it some time soon like next cooldown viz
Projects
None yet
Development

No branches or pull requests

3 participants