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(app): Disable reviewer links creation without a dataset version #2822

Merged
merged 3 commits into from
Jun 6, 2023

Conversation

nellh
Copy link
Contributor

@nellh nellh commented Jun 5, 2023

This disables the redirect to snapshot for reviewer users, allowing them to browse a draft. Also catches some rare cases where the redirect can trigger when snapshots are being created and avoids forcing a redirect before one exists by checking the tag value before attempting the redirect.

@nellh nellh requested a review from effigies June 5, 2023 21:26
@effigies
Copy link
Contributor

effigies commented Jun 6, 2023

I guess the question is whether we want to allow reviewers draft access. As an external reviewer, it would not be my goal to make the 1.0.0 release perfect, but to have something I can point to and say I reviewed it when it looked like this.

I think I'd rather disable reviewer links (or just give them sensible error messages) until the first snapshot.

@nellh nellh changed the title fix(app): Allow reviewers to browse drafts and snapshots fix(app): Disable reviewer links creation without a dataset version Jun 6, 2023
@nellh
Copy link
Contributor Author

nellh commented Jun 6, 2023

I guess the question is whether we want to allow reviewers draft access. As an external reviewer, it would not be my goal to make the 1.0.0 release perfect, but to have something I can point to and say I reviewed it when it looked like this.

I think I'd rather disable reviewer links (or just give them sensible error messages) until the first snapshot.

Went ahead and implemented that instead. This still includes the fix for the case which would cause the redirect to lead to a bad URL if the dataset has no snapshots but reviewers will still get redirected from draft to snapshot. Instead the reviewer link button is hidden with a warning to create a version first.

image

@effigies
Copy link
Contributor

effigies commented Jun 6, 2023

What happens to the reviewer links that already exist for a dataset without snapshots? Just looking at this, it seems like they'll be allowed to see the draft because the redirect doesn't get hit, but I might be misreading.

@nellh
Copy link
Contributor Author

nellh commented Jun 6, 2023

What happens to the reviewer links that already exist for a dataset without snapshots? Just looking at this, it seems like they'll be allowed to see the draft because the redirect doesn't get hit, but I might be misreading.

That's correct, the draft will be available.

Copy link
Contributor

@effigies effigies left a comment

Choose a reason for hiding this comment

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

Okay. This works. The dataset with reviewer links for the draft has now made a snapshot, so the redirect should work correctly going forward.

@nellh nellh merged commit dea6155 into master Jun 6, 2023
3 checks passed
@nellh nellh deleted the reviewer-draft-access branch June 6, 2023 13:16
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.

None yet

2 participants