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.

Sign up for GitHub

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

Jump to bottom

Fix delete response in admin users controller #4415

Merged
merged 1 commit into from
Jun 22, 2022
Merged

Fix delete response in admin users controller #4415

merged 1 commit into from
Jun 22, 2022

Conversation

tvdeyen
Copy link
Member

@tvdeyen tvdeyen commented Jun 14, 2022

Description

If a user with orders gets deleted in the admin an exception is raised. This exception is handled by the user_destroy_with_orders_error method.

This method uses the deprecated (and later removed) text response. Instead we need to use plain.

Checklist:

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have added tests to cover this change (if needed)

This was referenced Jun 14, 2022
Merged
Merged
Merged
jarednorman
jarednorman approved these changes Jun 14, 2022
View reviewed changes
Copy link
Member

@jarednorman jarednorman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah, :text doesn't work on Rails 5.1+.

@tvdeyen
Copy link
Member Author

tvdeyen commented Jun 15, 2022

Ah yeah, :text doesn't work on Rails 5.1+.

Yeah, and we didn't noticed...

@tvdeyen tvdeyen force-pushed the fix-user-destroy branch from c6f7eac to 55296cf Compare June 15, 2022 07:43
@waiting-for-dev
Copy link
Contributor

Hey @tvdeyen, could you rebase from master so that CI is green? 🙂

@tvdeyen
Fix delete response in admin users controller
0d641a7 
If a user with orders gets deleted in the admin an exception is raised. This exception is handled by the `user_destroy_with_orders_error method`.

This method uses the deprecated (and later removed) `text` response. Instead we need to use `plain`.
@tvdeyen tvdeyen force-pushed the fix-user-destroy branch from 55296cf to 0d641a7 Compare June 21, 2022 19:11
@tvdeyen
Copy link
Member Author

tvdeyen commented Jun 21, 2022

Hey @tvdeyen, could you rebase from master so that CI is green? 🙂

Done

waiting-for-dev
waiting-for-dev approved these changes Jun 22, 2022
View reviewed changes
Copy link
Contributor

@waiting-for-dev waiting-for-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@waiting-for-dev waiting-for-dev merged commit 7f7071c into solidusio:master Jun 22, 2022
@tvdeyen tvdeyen mentioned this pull request Jul 26, 2022
Closed
@tvdeyen tvdeyen linked an issue Jul 26, 2022 that may be closed by this pull request
Deleting a user with orders throws Missing Template exception #3126
Closed
@tvdeyen tvdeyen deleted the fix-user-destroy branch July 26, 2022 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jarednorman jarednorman jarednorman approved these changes

@waiting-for-dev waiting-for-dev waiting-for-dev approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Deleting a user with orders throws Missing Template exception
3 participants
@tvdeyen @waiting-for-dev @jarednorman