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

Ensure @relation_types is assigned before #update #97

Conversation

benjaminwil
Copy link
Contributor

When the admin variants controller cannot successfully complete an
#update action, it re-renders the user back on the #edit action
view.

This causes @relation_types to be nil and makes the Deface template
that tries to load product relations error out on a
@relation_types.empty? block.

We can avoid this by just making sure this before action is performed on
#update, too.

@benjaminwil
Copy link
Contributor Author

It looks like the recent execjs 2.7.0 update broke the test suite here, and we need to be on 2.8.1 as well as autoprefixer-rails 10.2.4.0. But I'm not sure how to explicitly do this.

@benjaminwil benjaminwil force-pushed the benjaminwil/fix-variants-controller-update-action branch 3 times, most recently from c110a71 to cd7f911 Compare May 19, 2021 22:55
@aldesantis
Copy link
Member

@benjaminwil looks like we're all good now! Can you update/remove your last commit?

@benjaminwil benjaminwil force-pushed the benjaminwil/fix-variants-controller-update-action branch 2 times, most recently from ae00cf6 to 05a3316 Compare May 20, 2021 19:46
@benjaminwil
Copy link
Contributor Author

@aldesantis When I removed my commit, the tests re-broke again. I know that we don't actually want to change any explicit dependencies in the gemspec here, so I'm not sure how to proceed. 🤔 Sorry.

@aldesantis
Copy link
Member

@benjaminwil actually, let's add that back in. I hadn't investigated the issue thoroughly, but it sounds like the only way forward is to upgrade the version of autoprefixer-rails. Sorry for the confusion!

@benjaminwil
Copy link
Contributor Author

I opened #98 so that I could better describe why I was making the change.

We can merge this after that one!

@aldesantis
Copy link
Member

@benjaminwil I merged your other PR, could you rebase? 🙏

When the admin variants controller cannot successfully complete an
`#update` action, it re-renders the user back on the `#edit` action
view.

This causes `@relation_types` to be nil and makes the Deface template
that tries to load product relations error out on a
`@relation_types.empty?` block.

We can avoid this by making sure this before action is performed on
`#update`, too.

Co-Authored-By: Adam Mueller <adam@super.gd>
@benjaminwil benjaminwil force-pushed the benjaminwil/fix-variants-controller-update-action branch from 05a3316 to a11f8f3 Compare May 24, 2021 18:46
@benjaminwil
Copy link
Contributor Author

Rebased!

@aldesantis
Copy link
Member

@benjaminwil thank you so much!

@aldesantis aldesantis merged commit e8472db into solidusio-contrib:master May 26, 2021
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