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

Adapt to 0.28 release and phone number enhancements #19

Merged
merged 42 commits into from
Jul 8, 2024

Conversation

Crashillo
Copy link
Contributor

@Crashillo Crashillo commented Apr 4, 2024

Closes https://github.com/PopulateTools/issues/issues/1926

  • Refactor views to fit for 0.28 release (redesign)
  • Use an specific telephone field for forms, in order to validate, via pattern, whether the phone number is valid or not. Such pattern can be edited by admins.
  • Allows admins also to edit the placeholder text which will appear in the users registration and edit account forms
  • Updates tests to 0.28 and adds new examples to check the phone number validation

Default pattern for spanish phonenumber: ^(\\+34)?[0-9 ]{9,12}$

Check the feature at https://decidim-redesign.populate.tools/admin/organization/edit

Pending

  • Persist to database the pattern chosen in the admin site
  • Place backend code properly
  • Add icon for the admin menu (engine.rb)

Screenshots

Admin:
imagen

Export Users feature:
Screenshot from 2024-04-17 16-01-21

Profile:
imagen

Register:
imagen

@furilo
Copy link
Member

furilo commented Apr 4, 2024

What patterns are allowed?
How are we explaining/documenting them?

@Crashillo
Copy link
Contributor Author

Crashillo commented Apr 4, 2024

Issue updated.

What patterns are allowed?

The pattern is a regex. It's pending yet to make work that field

How are we explaining/documenting them?

Current docs in this repo are technical. Is there anywhere else to publish a how to use this module?

@furilo
Copy link
Member

furilo commented Apr 4, 2024

Think from the admin perspective that is going to configure the field...

@Crashillo
Copy link
Contributor Author

Crashillo commented Apr 4, 2024

In that case, we can add help texts to the phone number pattern and phone number placeholder... although I think the fields are self-explanatory enough
imagen

@entantoencuanto entantoencuanto force-pushed the bump-0.28-phone-number branch from 236d197 to 7f3ebd7 Compare April 17, 2024 09:05
@entantoencuanto entantoencuanto marked this pull request as ready for review April 17, 2024 14:05
Copy link

@aramollis aramollis left a comment

Choose a reason for hiding this comment

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

https://www.loom.com/share/4a5500b5f1df4c9f9f392ff657a74550

  1. Only phone field has some kind of validation (you can define future date for birthday, and any character in CP...)
  2. In the admin config isn't easy to understand the validation rule ^(\+34)?[0-9 ]{9,12}$. This rule allow 3 extra digits +34999111222000 and numbers like 123456789 that are evident wrong spanish phone numbers
  3. If you deactivate extra user fields (not each field) the new fields disappear from signup form but not in edit profile form. For the profile you need to deactivate each field. https://www.loom.com/share/f210af6caa88440dbbac5e511842d275

I suggest at least

  • Improve validation phone rule to allow real phone numbers
  • Add this explanation in the admin config: Copia esta expresión "regex" ^(\+34)?[0-9 ]{9,12}$ para validar este formato de teléfono +34999111222
  • When deactivate extra user fields from admin config hide fields also from profile form

@furilo
Copy link
Member

furilo commented Apr 18, 2024 via email

@aramollis
Copy link

aramollis commented Apr 18, 2024

Aram define the various formats for the phone so the regex can be adjusted.

I'm not sure about. I found Plan nacional numeración 2021-02 Mnisterio Economía
I understand we should allow to start:

  • 0034 or +34 or ¿nothing?
  • First number should be only 9|8 for "fijos" and 6|7 for "móviles"
  • Second number couldn't be 0 they are reserved for paid services (Exception for 70 that are personal redirections to fijo/móvil)

Possible example of rules for fijo & móvil

Allowed examples

  • - +34 9## ### ###
  • - +34 8## ### ###
  • - +34 6## ### ###
  • - +34 7## ### ###
  • - 9## ### ###
  • - 8## ### ###
  • - 6## ### ###
  • - 7## ### ###

And not allowed

  • - 1-5*
  • - 0*
  • - Less than 9 digit numbers
  • +34 90*
  • +34 80*
  • 90*
  • 80*

@furilo
Copy link
Member

furilo commented Apr 18, 2024

I would go with the example of the URL:

(\+34|0034|34)?[ -]*(6|7)[ -]*([0-9][ -]*){8}

completing it so a phone can start by 8 and 9

@entantoencuanto
Copy link
Member

I have added a small change in the admin form to hide the phone number pattern and placeholder fields if the attribute is deactivated and hide the options for each field if the feature is globally disabled

Copy link

@aramollis aramollis left a comment

Choose a reason for hiding this comment

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

Now phone number validation works ok! 👏

@entantoencuanto entantoencuanto force-pushed the bump-0.28-phone-number branch from 3d729ad to b669e62 Compare April 23, 2024 18:29
@entantoencuanto entantoencuanto merged commit 5ba1c2f into main Jul 8, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants