@Crashillo or @entantoencuanto could you have a look on this PR ?

I'm giving a first look with the Product hat before doing a code review.

I have to say that this one is really difficult to review, because I can't make it work with the current code (aka the one from develop), as I want to check that we don't change/break anything.

For instance, I can't do the SMS check flow, as when I do it I have an error with redirections and cookies:

And if I disable the SMS checkbox in the Initiative Type settings page, then I have an alert saying that I can't sign:

Saying that, I tried it with this PR and seems like it works much better 🥳

A couple of errors/things that I've found. Feel free to move them to another issue if you think it's out of the scope of this, as I understand that the problem could be on other related components (such as decidim-verifications)

1. The SMS flow seems like it isn't show correctly (too much padding):

This is in Firefox 115.0

2. After signing there's a missing string interpolation

(Ignore the alert, as that's related to the next one 😄)

3. Missing authorization step

If I have the following settings in the Initiative Type:

And I don't have the "Identity documents" verification method in my participant account:

Then this step isn't shown, and it allows me to arrive to the Finish screen with an alert that says "The data provided to sign the initiative is not valid"

As far as I remember, this should be an step during the signing flow (or it should say to me something that I need to have this verification in my participant account). I'm not 100% sure on this one, as I can't check it out with the current code (develop), as it's buggy.

Just to be sure I tried it with others authorizations (Example authorization for instance) and I have the same behavior.

4. Authorization without SMS doesn't work

[This is actually happening too in the current code (develop), so its just probably the current (expected?) behavior, but I wanted to also add it here, so we can discuss it and move it to an issue if we think so.]

If I have an Initiative Type configured with the "Example authorization":

And there's an initiative associated, when I go to sign I have an error message:

It doesn't matter if I'm authorized with this verification method in the participant account or not, as the error message is always the same.

In the JS console and Rails logs I see an 422 error (422 Unprocessable Entity)

JS Console

XHRPOST
http://localhost:3000/initiatives/i-4/initiative_signatures
[HTTP/1.1 422 Unprocessable Entity 30ms]

Rails log

08:51:06 web.1 | Started POST "/initiatives/i-4/initiative_signatures" for 127.0.0.1 at 2023-08-24 08:51:06 +0200
08:51:06 web.1 | Decidim::Organization Load (0.3ms) SELECT "decidim_organizations".* FROM "decidim_organizations" WHERE "decidim_organizations"."host" = $1 LIMIT $2 [["host", "localhost"], ["LIMIT", 1]]
08:51:06 web.1 | Processing by Decidim::Initiatives::InitiativeSignaturesController#create as JS
08:51:06 web.1 | Parameters: {"authenticity_token"=>"[FILTERED]", "initiative_slug"=>"i-4"}
08:51:06 web.1 | Decidim::Initiative Load (0.4ms) SELECT "decidim_initiatives".* FROM "decidim_initiatives" WHERE "decidim_initiatives"."id" = $1 AND "decidim_initiatives"."decidim_organization_id" = $2 LIMIT $3 [["id", 4], ["decidim_organization_id", 1], ["LIMIT", 1]]
08:51:06 web.1 | Decidim::Organization Load (0.4ms) SELECT "decidim_organizations".* FROM "decidim_organizations" WHERE "decidim_organizations"."id" = $1 LIMIT $2 [["id", 1], ["LIMIT", 1]]
08:51:06 web.1 | Decidim::InitiativesTypeScope Load (0.4ms) SELECT "decidim_initiatives_type_scopes".* FROM "decidim_initiatives_type_scopes" WHERE "decidim_initiatives_type_scopes"."id" = $1 LIMIT $2 [["id", 9], ["LIMIT", 1]]
08:51:06 web.1 | Decidim::InitiativesType Load (0.4ms) SELECT "decidim_initiatives_types".* FROM "decidim_initiatives_types" WHERE "decidim_initiatives_types"."id" = $1 LIMIT $2 [["id", 3], ["LIMIT", 1]]
08:51:06 web.1 | Decidim::User Load (0.5ms) SELECT "decidim_users".* FROM "decidim_users" WHERE "decidim_users"."type" = $1 AND "decidim_users"."id" = $2 ORDER BY "decidim_users"."id" ASC LIMIT $3 [["type", "Decidim::User"], ["id", 2], ["LIMIT", 1]]
08:51:06 web.1 | (0.5ms) SELECT CONCAT('/pages/', slug) FROM "decidim_static_pages" WHERE "decidim_static_pages"."decidim_organization_id" = $1 AND "decidim_static_pages"."allow_public_access" = $2 ORDER BY "decidim_static_pages"."weight" ASC [["decidim_organization_id", 1], ["allow_public_access", true]]
08:51:06 web.1 | Decidim::InitiativesType Exists? (0.5ms) SELECT 1 AS one FROM "decidim_initiatives_types" INNER JOIN "decidim_initiatives_type_scopes" ON "decidim_initiatives_type_scopes"."decidim_initiatives_types_id" = "decidim_initiatives_types"."id" WHERE "decidim_initiatives_types"."decidim_organization_id" = $1 LIMIT $2 [["decidim_organization_id", 1], ["LIMIT", 1]]
08:51:06 web.1 | ===========
08:51:06 web.1 | :public
08:51:06 web.1 | :vote
08:51:06 web.1 | :initiative
08:51:06 web.1 | [Decidim::Initiatives::Permissions, Decidim::Admin::Permissions, Decidim::Permissions]
08:51:06 web.1 | ===========
08:51:06 web.1 | CACHE Decidim::Organization Load (0.0ms) SELECT "decidim_organizations".* FROM "decidim_organizations" WHERE "decidim_organizations"."id" = $1 LIMIT $2 [["id", 1], ["LIMIT", 1]]
08:51:06 web.1 | Decidim::InitiativesVote Exists? (0.7ms) SELECT 1 AS one FROM "decidim_initiatives_votes" WHERE "decidim_initiatives_votes"."decidim_initiative_id" = $1 AND "decidim_initiatives_votes"."decidim_author_id" = $2 LIMIT $3 [["decidim_initiative_id", 4], ["decidim_author_id", 2], ["LIMIT", 1]]
08:51:06 web.1 | CACHE Decidim::Organization Load (0.0ms) SELECT "decidim_organizations".* FROM "decidim_organizations" WHERE "decidim_organizations"."id" = $1 LIMIT $2 [["id", 1], ["LIMIT", 1]]
08:51:06 web.1 | Decidim::ResourcePermission Load (0.4ms) SELECT "decidim_resource_permissions".* FROM "decidim_resource_permissions" WHERE "decidim_resource_permissions"."resource_id" = $1 AND "decidim_resource_permissions"."resource_type" = $2 LIMIT $3 [["resource_id", 4], ["resource_type", "Decidim::Initiative"], ["LIMIT", 1]]
08:51:06 web.1 | Decidim::ResourcePermission Load (0.4ms) SELECT "decidim_resource_permissions".* FROM "decidim_resource_permissions" WHERE "decidim_resource_permissions"."resource_id" = $1 AND "decidim_resource_permissions"."resource_type" = $2 LIMIT $3 [["resource_id", 3], ["resource_type", "Decidim::InitiativesType"], ["LIMIT", 1]]
08:51:06 web.1 | Decidim::Scope Load (0.4ms) SELECT "decidim_scopes".* FROM "decidim_scopes" WHERE "decidim_scopes"."id" = $1 LIMIT $2 [["id", 1], ["LIMIT", 1]]
08:51:06 web.1 | Decidim::Authorization Load (0.5ms) SELECT "decidim_authorizations".* FROM "decidim_authorizations" LEFT OUTER JOIN "decidim_users" ON "decidim_users"."id" = "decidim_authorizations"."decidim_user_id" AND "decidim_users"."type" = $1 LEFT OUTER JOIN "decidim_organizations" ON "decidim_organizations"."id" = "decidim_users"."decidim_organization_id" WHERE "decidim_organizations"."id" = $2 AND "decidim_authorizations"."name" = $3 AND "decidim_authorizations"."decidim_user_id" = $4 ORDER BY "decidim_authorizations"."id" ASC LIMIT $5 [["type", "Decidim::User"], ["id", 1], ["name", "dummy_authorization_handler"], ["decidim_user_id", 2], ["LIMIT", 1]]
08:51:06 web.1 | CACHE Decidim::Authorization Load (0.0ms) SELECT "decidim_authorizations".* FROM "decidim_authorizations" LEFT OUTER JOIN "decidim_users" ON "decidim_users"."id" = "decidim_authorizations"."decidim_user_id" AND "decidim_users"."type" = $1 LEFT OUTER JOIN "decidim_organizations" ON "decidim_organizations"."id" = "decidim_users"."decidim_organization_id" WHERE "decidim_organizations"."id" = $2 AND "decidim_authorizations"."name" = $3 AND "decidim_authorizations"."decidim_user_id" = $4 ORDER BY "decidim_authorizations"."id" ASC LIMIT $5 [["type", "Decidim::User"], ["id", 1], ["name", "dummy_authorization_handler"], ["decidim_user_id", 2], ["LIMIT", 1]]
08:51:06 web.1 | Rendering /home/apereira/Work/decidim/decidim/decidim-initiatives/app/views/decidim/initiatives/initiative_signatures/error_on_vote.js.erb
08:51:06 web.1 | Rendered /home/apereira/Work/decidim/decidim/decidim-initiatives/app/views/decidim/initiatives/initiative_signatures/error_on_vote.js.erb (Duration: 0.1ms | Allocations: 22)
08:51:06 web.1 | Completed 422 Unprocessable Entity in 28ms (Views: 1.1ms | ActiveRecord: 5.4ms | Allocations: 31081)
08:51:06 web.1 |
08:51:06 web.1 |

@andreslucena , thanks for reviewing this. I will have an assessment and see if the mentioned issues are falling in the scope of this PR or not.

1. The SMS flow seems like it isn't show correctly (too much padding):

Viewing the site at 100% zoom level. I get to see the website as in the below picture. I could replicate the "to much padding", when i have visited the page using a zoom level of 70% - 80%.

2. After signing there's a missing string interpolation - Fixed

3. Missing authorization step - I think you may confuse the authorization screen, with the collection data screen
   

4. Authorization without SMS doesn't work

Actually, this seems to be an issue that comes from the authorization itself... I do not know how to fix it.
While investigating, i have reached to a point where the form (Decidim::Initiatives::VoteForm)) used by the command VoteInitiative returns an error like: :authorized_scopes=>["cannot be blank"], which is mapped to VoteInitiative.authorized_scopes.

      def authorized_scopes
        initiative.votable_initiative_type_scopes.select do |initiative_type_scope|
          initiative_type_scope.global_scope? ||
            initiative_type_scope.scope == user_authorized_scope ||
            initiative_type_scope.scope.ancestor_of?(user_authorized_scope)
        end.flat_map(&:scope)
      end

In my case, I have set the initiative as follows:

When i query from the console if the initiative.votable_initiative_type_scopes i get only one result, yet that scope responds false to '.global_scope?'. Global scope is actually checking that "decidim_scopes_id" is nil. Considering that initiative_type_scope.global_scope? is false, we get to the point where script can call user_authorized_scope .

def authorized_scope_candidates
        authorized_scope_candidates = [initiative.scope]
        authorized_scope_candidates += if initiative.scope.present?
                                         initiative.scope.descendants
                                       else
                                         initiative.organization.scopes
                                       end
        authorized_scope_candidates.uniq
      end

def user_authorized_scope
  return scope if handler_name.blank?
  return unless authorized?
  return if authorization.metadata.blank?

  @user_authorized_scope ||= authorized_scope_candidates.find do |scope|
    scope&.id == authorization.metadata.symbolize_keys[:scope_id]
  end
end

Considering that the authorization metadata for id_documents is empty, and i could not find a place where i could assign the scope to authorization, i would say, that the entire authorized_scopes returns empty result. Given that the tests are working, my first thought would be to check the seeds.

4. Authorization without SMS doesn't work (Part 2)

Defining a new initiative scope with only one global scope, made me successfully sign the document.

When trying using the "Collect participant personal data on signature" i have faced some issues. I have added my "identity documents" authorization, but when I went to sign the initiative with same document number i was unable. for some reason, i could not pass the collect data form, as there was a validation error on the document number.

@andreslucena Considering your input on this feature, i would say that we can start the Code review.
There is only one problem ( pct 2) that was fixed from my end.

The rest of the issues are not related to this PR, so we may start extracting them in separate tickets.

My analysis may not be 100% accurate, as i do not have enough experience with either initiatives or verifications module.

Most likely, we could have a more accurate answer whether the issues found are related to this refactor, or redesign or is just legacy, if we are going to test those scenarios in 0.27 release.

The SMS flow seems like it isn't show correctly (too much padding):
Viewing the site at 100% zoom level. I get to see the website as in the below picture. I could replicate the "to much padding", when i have visited the page using a zoom level of 70% - 80%.

@alecslupu I was testing this, is about the layout's grid, and I found a quick solution. I'll add a commit here.

The SMS flow seems like it isn't show correctly (too much padding):

@alecslupu I was testing this, is about the layout's grid, and I found a quick solution. I'll add a commit here.

I can confirm that this is fixed indeed. Thanks @Crashillo!

After signing there's a missing string interpolation - Fixed

I still see it. I also don't see any new commits from you. Maybe you forgot to push?

Missing authorization step - I think you may confuse the authorization screen, with the collection data screen

Yes, that was it. Once I checked the "Collect participant personal data on signature" it shows the form and it works as it should. Thanks for your help!

When trying using the "Collect participant personal data on signature" i have faced some issues. I have added my "identity documents" authorization, but when I went to sign the initiative with same document number i was unable. for some reason, i could not pass the collect data form, as there was a validation error on the document number.

I think that the happy path for this personal data collection to work is to use it with the "Example authorization". At least with that one I was able to make it work (adding the same data in the authorization and in the data collection form).

I can confirm that it isn't working with the "Identity documents" authorization

So, as all my feedback with the Product hat is finished, I think I can start with the code review. Can you first check out the string interpolation thing @alecslupu so we're sure that all the code is pushed? Thanks

@andreslucena , the string interpolation is fixed by 2969617

As I mentioned at the beginning, I'm OK with merging this PR as-is, just wanted your opinion on the first point (if we want to make it consistent on the update/store naming)

Both changes are being implemented in #11545

Both changes are being implemented in #11545

Great! I'll merge this one, so we can keep moving then 👍🏽