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

Avoid exceptions in Management section #4443

Merged
merged 2 commits into from
Apr 7, 2021
Merged

Avoid exceptions in Management section #4443

merged 2 commits into from
Apr 7, 2021

Conversation

taitus
Copy link
Member

@taitus taitus commented Mar 25, 2021

References

Closes #2767

Objectives

Many management actions only make sense if a user has been selected beforehand.
The objective is only allow management actions when we have a selected user.

Notes

I was unable to reproduce the error in the related issue (#2767) with the steps above.
But I have detected a bug related and even partially commented on in some comments of that issue.

@taitus taitus added the Bug label Mar 25, 2021
@taitus taitus self-assigned this Mar 25, 2021
@taitus taitus added this to Reviewing in Consul Democracy via automation Mar 25, 2021
@taitus taitus moved this from Reviewing to Doing in Consul Democracy Mar 25, 2021
Consul Democracy automation moved this from Doing to Testing Apr 6, 2021
@Senen Senen self-assigned this Apr 6, 2021
Copy link
Member

@javierm javierm left a comment

Choose a reason for hiding this comment

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

@taitus Great we're finally dealing with this part of the code 😄.

How about adding a test for another scenario? Users go to "Print proposals" and click the "Support" button in one of the proposals.

app/controllers/management/base_controller.rb Outdated Show resolved Hide resolved
config/locales/en/management.yml Outdated Show resolved Hide resolved
config/locales/es/management.yml Outdated Show resolved Hide resolved
spec/system/management/account_spec.rb Outdated Show resolved Hide resolved
spec/system/management/account_spec.rb Show resolved Hide resolved
spec/system/management/budget_investments_spec.rb Outdated Show resolved Hide resolved
spec/system/management/budget_investments_spec.rb Outdated Show resolved Hide resolved
spec/system/management/proposals_spec.rb Outdated Show resolved Hide resolved
@javierm javierm moved this from Testing to Doing in Consul Democracy Apr 7, 2021
@taitus taitus force-pushed the improve-management branch 4 times, most recently from 0330f8c to e7cdd40 Compare April 7, 2021 16:46
We prepare the file to be able to include specs
that do not need to have a logged-in user.

We also took the opportunity to not execute this
line in some specs where it was not necessary.
@taitus taitus moved this from Doing to Reviewing in Consul Democracy Apr 7, 2021
Many management actions only make sense if a user has been selected
beforehand.

We updated :check_verified_user method to be able to check  actions that need to
have a user selected in order to avoid exceptions.

We need this control as :only_verified_user is not restrictive enough. The reason is
that the :managed_user method used in the :only_verified_user if it does not find a
user it does an initializce (find_or_initialize_by). This causes that when we have
"skip_verification" to true, it returns this non-persisted user as "verified".

These changes affect the actions of Account, Budgets and Proposals Controller
when no user is selected.
@javierm
Copy link
Member

javierm commented Apr 7, 2021

The test which failed is not related to this pull request and will be fixed by #4467.

@javierm javierm merged commit 214bb0f into master Apr 7, 2021
Consul Democracy automation moved this from Reviewing to Release 1.3.0 Apr 7, 2021
@javierm javierm deleted the improve-management branch April 7, 2021 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error 500 supporting proposals as a manager
3 participants