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

Edit Budget Investment only in accepting phase #3716

Merged
merged 12 commits into from
Oct 18, 2019
Merged

Edit Budget Investment only in accepting phase #3716

merged 12 commits into from
Oct 18, 2019

Conversation

denialtorres
Copy link
Contributor

@denialtorres denialtorres commented Sep 22, 2019 •
edited by javierm
Loading

References

resolves #3691

let the users modify his budget investments only in the accepting phase

Visual Changes

The Edit option will appear under the Delete button if they user goes to My Content >> Investments only during the Accepting Phase, after that the button will disappear

image

if they try to go to the edit url during another phase or without being the author of the proposal then they will be rejected

Screen Shot 2019-09-22 at 3 50 44 PM

Notes

this is only working to regular users, if you're an admin you still will be able to edit your proposals in the following phases

houndci-bot
houndci-bot reviewed Sep 22, 2019
View reviewed changes
config/routes/budget.rb Outdated Show resolved Hide resolved
app/views/budgets/investments/_form_edit.html.erb Outdated Show resolved Hide resolved
app/controllers/budgets/investments_controller.rb Outdated Show resolved Hide resolved
app/controllers/budgets/investments_controller.rb Outdated Show resolved Hide resolved
app/controllers/budgets/investments_controller.rb Outdated Show resolved Hide resolved
@denialtorres denialtorres changed the title WIP: Edit Budget Investment only in accepting phase Edit Budget Investment only in accepting phase Sep 22, 2019
@javierm javierm added this to Reviewing in Roadmap via automation Sep 22, 2019
Roadmap automation moved this from Reviewing to Doing Sep 23, 2019
javierm
javierm reviewed Sep 30, 2019
View reviewed changes
Copy link
Member

@javierm javierm left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@denialtorres Thanks a lot for this pull request! In my humble opinion this is a very useful feature 😄.

I've made a few comments. Let me know what you think! I might be wrong on some things, so feel free to disagree 😉.

app/controllers/budgets/investments_controller.rb Outdated Show resolved Hide resolved
app/models/abilities/common.rb Outdated Show resolved Hide resolved
app/controllers/budgets/investments_controller.rb Outdated Show resolved Hide resolved
app/views/budgets/investments/edit.html.erb Outdated Show resolved Hide resolved
config/routes/budget.rb Outdated Show resolved Hide resolved
spec/models/abilities/common_spec.rb Show resolved Hide resolved
spec/models/abilities/common_spec.rb Show resolved Hide resolved
@javierm javierm moved this from Doing to Reviewing in Roadmap Oct 1, 2019
@javierm javierm moved this from Reviewing to Doing in Roadmap Oct 1, 2019
@javierm javierm moved this from Doing to Reviewing in Roadmap Oct 5, 2019
@javierm javierm added the Budgets label Oct 5, 2019
houndci-bot
houndci-bot reviewed Oct 7, 2019
View reviewed changes
spec/features/budgets/investments_spec.rb Outdated Show resolved Hide resolved
spec/features/budgets/investments_spec.rb Outdated Show resolved Hide resolved
spec/features/budgets/investments_spec.rb Outdated Show resolved Hide resolved
spec/features/budgets/investments_spec.rb Outdated Show resolved Hide resolved
spec/features/budgets/investments_spec.rb Outdated Show resolved Hide resolved
@denialtorres
Copy link
Contributor Author

hi @javierm, it seems to be ready now

do you know why i keep getting errors in the coveralls part ?? regards!

@javierm
Copy link
Member

javierm commented Oct 7, 2019

Great! I'll have a look at the changes during the week.

Lately coveralls integration with our Travis builds isn't working so well 🤔. I've just disabled it, since as it is right now it's very confusing.

javierm
javierm reviewed Oct 9, 2019
View reviewed changes
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.

@denialtorres Thank you so much for your work on this pull request! I've left a few comments; it looks like there are a couple of issues when the submitted data isn't valid. Let me know what you think!

app/views/budgets/investments/edit.html.erb Outdated Show resolved Hide resolved
app/views/users/_budget_investment.html.erb Outdated Show resolved Hide resolved
config/locales/en/management.yml Show resolved Hide resolved
spec/features/budgets/investments_spec.rb Outdated Show resolved Hide resolved
app/views/budgets/investments/_form.html.erb Outdated Show resolved Hide resolved
app/controllers/budgets/investments_controller.rb Outdated Show resolved Hide resolved
@javierm javierm moved this from Reviewing to Doing in Roadmap Oct 9, 2019
@javierm javierm self-assigned this Oct 12, 2019
@denialtorres
Copy link
Contributor Author

hey, hi @javierm the changes are ready, can you check them when you have a chance?, you were right in the part of the form, it is much better if I put budget_investment_path (@budget, @investment in the edit view.

also i add a new scenario to activate the validation error and make sure that part is working

@javierm javierm moved this from Doing to Reviewing in Roadmap Oct 14, 2019
Roadmap automation moved this from Reviewing to Testing Oct 18, 2019
javierm
javierm approved these changes Oct 18, 2019
View reviewed changes
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.

@denialtorres Thanks a lot! 😄

@javierm javierm merged commit bb627a7 into consuldemocracy:master Oct 18, 2019
Roadmap automation moved this from Testing to Release 1.1.0 Oct 18, 2019
@javierm javierm mentioned this pull request Oct 18, 2019
Merged
@microweb10 microweb10 mentioned this pull request Feb 4, 2020
Merged
smarques pushed a commit to venetochevogliamo/consul that referenced this pull request Apr 29, 2020
@denialtorres @javierm
Edit Budget Investment only in accepting phase (consuldemocracy#3716)
1f1a85f 
This way users who made a typo can fix it before the investment is reviewed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@houndci-bot houndci-bot houndci-bot left review comments

@PierreMesure PierreMesure PierreMesure requested changes

@javierm javierm javierm approved these changes

Assignees

@javierm javierm

@denialtorres denialtorres

Labels
Budgets
Projects
No open projects
Roadmap
  
Release 1.1.0
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Let the users edit his budget investment proposals only during the acceptance phase
4 participants
@denialtorres @javierm @houndci-bot @PierreMesure