Refactor intiative wizard by alecslupu · Pull Request #10727 · decidim/decidim

alecslupu · 2023-04-17T12:44:56Z

🎩 What? Why?

This PR improves the Create initiative wizard, by removing the session support in favor of the database support.

📌 Related Issues

Link your PR to an issue

Testing

Visit the admin panel , and enable the WYSIWYG for public participants
Configure the initiatives
Create an initiative.
See that you get an error
Apply patch
Create an initiative
See there is no error.

📷 Screenshots

Please add screenshots of the changes you are proposing

♥️ Thank you!

…itiative-creation

alecslupu · 2023-04-24T15:32:30Z

@decidim/maintainers , how should i label this ? Shall we consider it a fix, or a change?

…itiative-creation

andreslucena · 2023-05-23T12:07:31Z

how should i label this ? Shall we consider it a fix, or a change?

Let's review it first and once we all are OK with the final form let's talk about that, as it could be difficult to backport (and that's the main difference between type: change and type: fix at the moment).

Great change! This is the typical bad decision made years ago that we had to change sometime ago, so I'm glad to actually see this going forward 😄

I have some changes to request that I found doing the comparison between how it worked and how it works now:

Remove scope selector from Start step (/create_initiative/previous_form)

v0.27	this PR

Scope selector isn't translated
Missing type selector in Create step (/create_initiative/fill_data)

v0.27	this PR

…itiative-creation

alecslupu · 2023-05-24T12:24:38Z

@andreslucena, suggestions applied. Can you have a look?

andreslucena

I've finished a first check of the code and I have some suggestions and doubts, can you check them out please?

decidim-initiatives/app/models/decidim/initiative.rb

decidim-initiatives/app/commands/decidim/initiatives/create_initiative.rb

decidim-initiatives/app/controllers/decidim/initiatives/create_initiative_controller.rb

decidim-initiatives/spec/shared/create_initiative_example.rb

Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>

…idim into refactor/initiative-creation

…itiative-creation

Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>

…idim into refactor/initiative-creation

…itiative-creation

alecslupu · 2023-05-29T09:40:32Z

@andreslucena this can be reviewed. There are some comments left unresolved for you to see as there i had some different approach on various things

andreslucena · 2023-05-30T08:35:39Z

how should i label this ? Shall we consider it a fix, or a change?

After seeing that the change is quite big, and as this apart from fixing some bugs could introduce some new bugs, I'd prefer to leave this as a type: change and not backport this.

I guess that the real issue is that I'm not confident in the current test suite regarding all the combination of InitiativeTypes options.

andreslucena

Just tried one more time locally and everything looks good 👏🏽 👏🏽
Thanks @alecslupu for handling this long-standing issue.
As it's a big change, lets wait for @ahukkanen review and approval before merging this

ahukkanen

I have gone through these changes and tested the changes from the user perspective. The bug that these refactoring efforts are fixing is gone and initiative creation now works fine even with longer descriptions. 👍

Glancing through the code, I don't have much comments, only one potential bug I noticed that reminded me about an older bug. I added a small comment regarding that.

Other than that this is all good for me.

I also must say that I am not an expert with initiatives, so I would definitely trust @andreslucena review much more than mine. For me this module has always been very hard to understand as we haven't had any actual use for it in our instances, so I haven't put much effort into understanding this module myself. I can still see that the code is quite complex and overwhelming, so there must be many more places for refactoring there but this is already a great start with a nice bugfix. Great work!

decidim-initiatives/app/commands/decidim/initiatives/create_initiative.rb

Co-authored-by: Antti Hukkanen <antti.hukkanen@mainiotech.fi>

ahukkanen

Everything is fine for me!

I'll let @andreslucena to make the final merge as he put most of the effort in reviewing this (and because the latest commit dismissed his previous review).

andreslucena

👍🏽

* Refactor intiative wizard * Fix the initiative creation * Fixing failing specs * Apply review recommendations * Update decidim-initiatives/app/models/decidim/initiative.rb Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com> * Apply review recommendations * Fix the user group creation * Small refactor on initiatives * Add spec for custom signature end date in update command * Update decidim-initiatives/app/controllers/decidim/initiatives/create_initiative_controller.rb Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com> * Update decidim-initiatives/lib/decidim/initiatives/engine.rb Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com> * Apply latest review recommendations * Add area spec * Apply suggestions from code review Co-authored-by: Antti Hukkanen <antti.hukkanen@mainiotech.fi> --------- Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com> Co-authored-by: Antti Hukkanen <antti.hukkanen@mainiotech.fi>

alecslupu added the module: initiatives label Apr 17, 2023

alecslupu added 2 commits April 18, 2023 16:43

Refactor intiative wizard

f0f6e17

Fix the initiative creation

c012e6d

alecslupu force-pushed the refactor/initiative-creation branch from 5ab6922 to c012e6d Compare April 18, 2023 15:07

Fixing failing specs

9972252

alecslupu mentioned this pull request Apr 19, 2023

Refactor initiative signing wizard #10731

Merged

Merge branch 'develop' of github.com:decidim/decidim into refactor/in…

8cb99cf

…itiative-creation

Merge branch 'develop' of github.com:decidim/decidim into refactor/in…

29c46ff

…itiative-creation

alecslupu marked this pull request as ready for review April 26, 2023 09:16

andreslucena requested review from ahukkanen and andreslucena May 3, 2023 09:40

Merge branch 'develop' of github.com:decidim/decidim into refactor/in…

cdce5cc

…itiative-creation

alecslupu force-pushed the refactor/initiative-creation branch from 2ffeb2c to cdce5cc Compare May 6, 2023 11:16

Merge branch 'develop' of github.com:decidim/decidim into refactor/in…

6aa0cd6

…itiative-creation

Merge branch 'develop' of github.com:decidim/decidim into refactor/in…

83051c3

…itiative-creation

alecslupu force-pushed the refactor/initiative-creation branch 2 times, most recently from 157f090 to a180a1b Compare May 24, 2023 10:58

Apply review recommendations

13531f5

alecslupu force-pushed the refactor/initiative-creation branch from a180a1b to 13531f5 Compare May 24, 2023 11:07

andreslucena requested changes May 25, 2023

View reviewed changes

alecslupu and others added 5 commits May 25, 2023 12:36

Update decidim-initiatives/app/models/decidim/initiative.rb

95d755c

Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>

Apply review recommendations

c20710e

Merge branch 'refactor/initiative-creation' of github.com:decidim/dec…

3995999

…idim into refactor/initiative-creation

Merge branch 'develop' of github.com:decidim/decidim into refactor/in…

f5ecfd3

…itiative-creation

Fix the user group creation

2d08994

alecslupu requested a review from andreslucena May 25, 2023 13:10

Small refactor on initiatives

f3e93a0

alecslupu and others added 4 commits May 26, 2023 11:45

Update decidim-initiatives/lib/decidim/initiatives/engine.rb

57726df

Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>

Merge branch 'refactor/initiative-creation' of github.com:decidim/dec…

a136837

…idim into refactor/initiative-creation

Apply latest review recommendations

36ec789

Merge branch 'develop' of github.com:decidim/decidim into refactor/in…

3a806a6

…itiative-creation

alecslupu force-pushed the refactor/initiative-creation branch from 099f6c1 to 3a806a6 Compare May 26, 2023 14:59

alecslupu mentioned this pull request May 26, 2023

Remove unused preset-env dependencies #10916

Merged

Merge branch 'develop' of github.com:decidim/decidim into refactor/in…

06c1cf5

…itiative-creation

andreslucena mentioned this pull request May 29, 2023

Fix for initiative menu not active on creation #10919

Merged

Add area spec

b6a6d01

alecslupu requested a review from andreslucena May 30, 2023 08:27

alecslupu added the type: change PRs that implement a change for an existing feature label May 30, 2023

andreslucena previously approved these changes May 30, 2023

View reviewed changes

ahukkanen requested changes May 30, 2023

View reviewed changes

decidim-initiatives/app/commands/decidim/initiatives/create_initiative.rb Outdated Show resolved Hide resolved

alecslupu dismissed andreslucena’s stale review via af34f7f May 30, 2023 13:45

Apply suggestions from code review

af34f7f

Co-authored-by: Antti Hukkanen <antti.hukkanen@mainiotech.fi>

ahukkanen approved these changes May 30, 2023

View reviewed changes

andreslucena approved these changes May 31, 2023

View reviewed changes

andreslucena merged commit 8169381 into develop May 31, 2023

andreslucena deleted the refactor/initiative-creation branch May 31, 2023 06:45

alecslupu mentioned this pull request Jun 12, 2023

Redesign: initiatives #10646

Merged

2 tasks

alecslupu mentioned this pull request Jul 11, 2023

Remove unused dependency: wicked #11150

Merged

alecslupu mentioned this pull request Sep 26, 2023

Initiative create and update does not process all attachments #11653

Closed

alecslupu mentioned this pull request Feb 20, 2024

Cookie Overflow when creating an initiative with long description #12464

Closed

fblupi mentioned this pull request Feb 23, 2024

Bump decidim version AjuntamentdeBarcelona/decidim-barcelona#531

Merged

fblupi mentioned this pull request Feb 27, 2024

release/0.27 stable bcn AjuntamentdeBarcelona/decidim#34

Draft

alexrlpz mentioned this pull request Nov 4, 2024

Create initiative form is not taking into account the scope selected #13597

Closed

Uh oh!

Conversation

alecslupu commented Apr 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎩 What? Why?

📌 Related Issues

Testing

📷 Screenshots

Uh oh!

alecslupu commented Apr 24, 2023

Uh oh!

andreslucena commented May 23, 2023 • edited by alecslupu Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alecslupu commented May 24, 2023

Uh oh!

andreslucena left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alecslupu commented May 29, 2023

Uh oh!

andreslucena commented May 30, 2023

Uh oh!

andreslucena left a comment

Choose a reason for hiding this comment

Uh oh!

ahukkanen left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ahukkanen left a comment

Choose a reason for hiding this comment

Uh oh!

andreslucena left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alecslupu commented Apr 17, 2023 •

edited

Loading

andreslucena commented May 23, 2023 •

edited by alecslupu

Loading