🧹Marketplace: Remove unused TaxRate partial!
#1352
Conversation
- #1137 Co-authored-by: Kelly Hong <KellyAH@users.noreply.github.com
- #1187 Turns out when we are testing `remove` turbo streams it was trying to render a partial for the thing to remove. That's A) unnecessary and B) a recipe for cruft because the inferred rendering will require partials named for each model; and those partials may not need to exist if the particular leaf is done using ViewComponent
| @@ -25,6 +25,7 @@ def initialize(action, target, content = nil, turbo_stream:, **rendering, &callb | |||
| def matches?(response) | |||
| @response = response | |||
| assert_turbo_stream(action: action, target: target) do | |||
| rendering[:allow_inferred_rendering] = false if action == :remove | |||
There was a problem hiding this comment.
This change seems unrelated to the description and the rest of this PR. I assume you need this change for something else somewhere else and just decided to pull it into this PR, and this is not a mistake?
There was a problem hiding this comment.
Yea, the second commit has more details, but basically assert_turbo_stream presumes you want to :allow_inferred_rendering, so when I removed this partial the TaxRatesController#destroy tests started failing because it wanted to render. When you look at the underlying code for the TurboStreamBuilder, however, the remove action when called directly sets allow_inferred_rendering to false.
I considered calling send(action, ...) so that we would be using the TurboStreamBuilders defaults, but ran into some weirdness that I didn't want to take time to pin down.
There was a problem hiding this comment.
My point was not about the implementation, but about the fact that this change does not seem to belong in this PR.
There was a problem hiding this comment.
Ah, well the reason it is in this PR was because the test was failing when I removed the partial.
Marketplace:TaxCollection and Compliance #1137This also makes our
have_rendered_turbo_streammatcher play more nicely withremoveactions.Co-authored-by: Kelly Hong <KellyAH@users.noreply.github.com