Feedback needed after Endorsing when user has no user_groups#2998
Feedback needed after Endorsing when user has no user_groups#2998josepjaume merged 14 commits intomasterfrom
Conversation
|
@tramuntanal the i18n tests are failing, can you check them please? |
Codecov Report
@@ Coverage Diff @@
## master #2998 +/- ##
==========================================
+ Coverage 98.67% 98.67% +<.01%
==========================================
Files 1698 1699 +1
Lines 40500 40568 +68
==========================================
+ Hits 39964 40032 +68
Misses 536 536 |
mrcasals
left a comment
There was a problem hiding this comment.
Code looks good to me! Can we have some system spec checking this? I'm pretty sure there's already some test checking the Endorse button works as expected, modify that spec the check that the button text is changed please 😄
|
|
||
| require "spec_helper" | ||
|
|
||
| describe "Endorse Proposal", type: :system do |
There was a problem hiding this comment.
So we had no system specs for this? 🤔 I think it's good we're adding them then! 😄
|
@tramuntanal some tests fail, can you check them please? Also, you might need to rebase against |
| @@ -1,4 +1,10 @@ | |||
| <% fully_endorsed= fully_endorsed?(proposal, current_user) %> | |||
| <%# main page button %> | |||
There was a problem hiding this comment.
Sorry to be such a PITA over this, but we're not using comments like this anywhere in the project. We could start a discussion whether that's needed or it actually reflects a design flaw (I'm more inclined to that last argument), but for the moment, let's comply with the current style please :)
There was a problem hiding this comment.
Or this means this part could be extracted in a partial/cell 😄
There was a problem hiding this comment.
I agree that it is actually a hint for a design flaw. I extracted the code into js methods.
…eeded_after_endorsing_wo_user_groups
|
Hi! I've merged from master since the build wasn't passing due to a bug that's already fixed in there. |
…' of github.com:decidim/decidim into fix/2968-feedback_needed_after_endorsing_wo_user_groups
|
@decidim/lot-core It seems that last merge from master broke the specs. It seems a problem from master, can you check please? |
|
@tramuntanal rebasing against |
* master: [RFC] Use cells for meeting m cards (#3022) Do not force Postgresql user to be admin when enabling trigram extension (#3053) Make organization reference_prefix required (#3056) admin can duplicate/copy meetings (#3051) Fix question form errors not being displayed (#3046) Erb whitespace cutting (#3047) Show debates statistics on space show and homepage (#3016) Fix broken translated field after form errors (#3026) Move decidim executable to "exe" folder (#3028) Friendlier buttons (#3027) Feedback needed after Endorsing when user has no user_groups (#2998) Fix seeding error on generator specs (#3021) fix spelling error in threshold (#3019) Migration plus seeds (#2933)
…new design.
🎩 What? Why?
Javascript to update endorsement button was lost when applying new design.
📌 Related Issues
📋 Subtasks
CHANGELOGentry