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 /task/:id to redirect to deploys/tasks/rollbacks #886

Merged
merged 3 commits into from Apr 9, 2019

Conversation

Projects
None yet
4 participants
@esnunes
Copy link
Contributor

esnunes commented Apr 3, 2019

Problem

At the moment the user needs to know the stack path in order to create a link to a task, deploy or rollback, even with the given resource id at hand.

Solution

Create the endpoint /task/:id that with the given resource id, redirects the user to the correct resource url.

Notes

Rollback extends Deploy but doesn't have a specific view/url, because of that the code transforms any subclass of Deploy to look like a Deploy using ActiveRecord#becomes method.

@esnunes esnunes added the enhancement label Apr 3, 2019

@esnunes esnunes self-assigned this Apr 3, 2019

@esnunes esnunes requested review from DazWorrall and JackTLi Apr 3, 2019

@DazWorrall DazWorrall requested a review from casperisfine Apr 3, 2019

assert_redirected_to stack_deploy_path(@task.stack, @task)
end

test ":lookup returns stack deploy url if the task is a kind of Deploy" do

This comment has been minimized.

Copy link
@DazWorrall

DazWorrall Apr 3, 2019

Member

nit kind of Rollback?

This comment has been minimized.

Copy link
@esnunes

esnunes Apr 3, 2019

Author Contributor

actually not, the business logic I want to test is: given the instance from a subclass of Deploy
it should redirect to the deploy resource url, in this example I'm providing a Rollback instance.

I can change to :lookup returns stack deploy url if the task is an instance of a subclass of Deploy, what do you think?

This comment has been minimized.

Copy link
@JackTLi

JackTLi Apr 3, 2019

Member

From the test perspective, we should forget that any subclassing exists. This test is explicitly testing that Rollbacks receive a deploy url, so we should probably label it like that. If we get more subclasses in the future, they can all get their own test cases too 🙂

This comment has been minimized.

Copy link
@esnunes

esnunes Apr 3, 2019

Author Contributor

it makes sense, I will change that

@JackTLi

JackTLi approved these changes Apr 3, 2019

@esnunes esnunes force-pushed the task-lookup branch from 672f66a to 09be6c5 Apr 3, 2019

@JackTLi

JackTLi approved these changes Apr 4, 2019

@doodzik

doodzik approved these changes Apr 8, 2019

@esnunes esnunes merged commit 0bb8986 into master Apr 9, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@esnunes esnunes deleted the task-lookup branch Apr 9, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.