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

Redirect back after discarding a job #157

Merged
merged 1 commit into from
Oct 31, 2024

Conversation

thiagobc
Copy link
Contributor

@thiagobc thiagobc commented Sep 9, 2024

Discarding a job can currently be done from "Failed jobs" and "Scheduled jobs" pages, but, as a result, the user is always redirected to "Failed jobs" page.

This change will make the user be redirected back instead.

@rosa
Copy link
Member

rosa commented Oct 31, 2024

Thank you @thiagobc, and sorry for the delay! I'be been busy with other stuff and haven't had time to look at this gem, but I'm catching up now.

@rosa rosa merged commit 04d66fd into rails:main Oct 31, 2024
@rosa
Copy link
Member

rosa commented Oct 31, 2024

Ahh, this breaks the case of discarding a job from its individual page. It'd try to redirect to the job page, but it'll get a 404 error because the job is gone. I'll fix it.

rosa added a commit that referenced this pull request Oct 31, 2024
This is a follow-up to #157, which redirected back after discarding
instead of redirecting to the failed job page always, since you can
discard a job from the scheduled jobs page as well. However, this
doesn't work in the case of discarding a job from its individual page,
because in that case we'd try to redirect to the job, and the job no
longer exists, which results in a 404.

With this change, we just redirect to wherever the status of the
discarded job is.
rosa added a commit that referenced this pull request Oct 31, 2024
This is a follow-up to #157, which redirected back after discarding
instead of redirecting to the failed job page always, since you can
discard a job from the scheduled jobs page as well. However, this
doesn't work in the case of discarding a job from its individual page,
because in that case we'd try to redirect to the job, and the job no
longer exists, which results in a 404.

With this change, we just redirect to wherever the status of the
discarded job is.
@thiagobc
Copy link
Contributor Author

Hi @rosa! Sorry for breaking the pipeline, I missed running bin/rails app:test:system before creating the PR. 🤦

Anyway, thanks to you for the great project and for fixing my changes 🙏. (that was a smart fix by the way.)

@thiagobc thiagobc deleted the discard-redirect-back branch October 31, 2024 16:58
@rosa
Copy link
Member

rosa commented Oct 31, 2024

Oh, no worries at all, not your fault! I'm not sure why the tests didn't run automatically as part of your PR... they should have! You shouldn't need to remember that. Thanks a lot for your fix and your kind words!

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