Shopify / Shopify/shipit-engine
Stack page links deploys/rollbacks to tasks#show, which 500s on @task.definition.id (nil for deploys)
Nobody has claimed this yet.
- Dominant language
- Ruby
- Stars
- 1.5k
- Forks
- 154
- Avg merge
- 2h 26m
- Merged PRs (30d)
- 3
Description
Summary
On a stack's show page, every entry in the Previous Deploys list (including Shipit::Deploy and Shipit::Rollback records) is linked to tasks#show (/stacks/:id/tasks/:task_id). Clicking on a "deploy" or "rollback" task that way raises:
ActionView::Template::Error (undefined method 'id' for nil)
app/views/shipit/tasks/show.html.erb:2
so clicking any deploy in Previous Deploys returns a 500.
Note that overriding the link and visiting the same record at /stacks/:id/deploys/:id works fine.
Steps to reproduce
- Open a stack that has previous deploys.
- Click a deploy (or rollback) in the Previous Deploys list — it links to
/stacks/:id/tasks/:task_id. - 500:
undefined method 'id' for nilatapp/views/shipit/tasks/show.html.erb:2.
Root cause
tasks/show.html.erb renders the task title with @task.definition.id:
<%= @task.active? ? "executing" : "executed" %> <%= @task.definition.id %>
definition is a serialized column populated only for generic shipit.yml tasks.
Deploys and rollbacks don't seem to ever have one (Stack#build_deploy doesn't set it).
This view is only meant for generic tasks — deploys/rollbacks have their own deploys/show and deploys/_deploy. The reason they now reach tasks#show is the deploy list in stacks/show.html.erb:
<%= render partial: 'shipit/tasks/task', collection: @tasks %>
This forces every record through shipit/tasks/_task, which links via stack_task_path → tasks#show. Previously the list used polymorphic rendering:
<%= render @tasks %>
which dispatched each record via to_partial_path: Shipit::Deploy → shipit/deploys/_deploy and Shipit::Rollback (to_partial_path overridden to 'deploys/deploy') → the same deploy partial, both linking to deploys#show. Only generic tasks rendered tasks/_task.
The change came in 793e1d5cc22fece0eb644160026042a483dc1f49, which replaced render @tasks with the explicit single-partial form.
That seemed to have suppressed the implicit-render warning but loses the per-type partial dispatch for the STI subclasses.
Still present on main / 0.45.1.
Possible fixes
- Restore polymorphic rendering (
render @tasks) for the deploy list instacks/show.html.erb; or - Keep explicit rendering but dispatch per type (
deploys/_deployforDeploy/Rollback,tasks/_taskfor generic tasks); or - Make
tasks/show.html.erbtolerate a nildefinition(e.g. fall back to@task.title) so a deploy routed there doesn't crash.
Environment
- shipit-engine 0.45.1 (also reproducible on 0.44.2)
- Rails 8.1.3
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start with app/views/stacks/show.html.erb and compare its rendering of @tasks with the type-specific partials app/views/shipit/tasks/_task and app/views/shipit/deploys/_deploy. Read app/views/shipit/tasks/show.html.erb and reproduce the Previous Deploys navigation; done means deploys and rollbacks no longer reach the generic task view or produce a 500, while generic tasks still work.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rails, ruby
- Domain
- backend, devops
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 72/100