1 feature request migrate hardcoded states dropdown

[grayeul]

Description & Issue number it closes

This should be considered a DRAFT Pull Request for Issue #6644. It was chosen because of the 'good first issue' tag, and I was looking for a relatively simple fix that would help me navigate the waters for future contributions. The main purpose of this fix is to consolidate where State DropDown options are presented, and use them in a consistent way.

Screenshots (if appropriate)

I don't think this (as it currently stands...) should affect anything related to screenshots. Though there could be later changes after discussions that might make some small changes.

How to test the changes?

What I've been doing to test this is just going into the view of "Active People" and selecting someone to edit, which goes through the PersonEditor.php. With the 'old' hard-coded way from the original StateDropDown.php, the list of States provided (for US) started with Alabama, Alaska, Arizona..... After the change, you should see: Alabama, Alaska, American Samoa, ... as this comes from the list in src/locale/states/us.json

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• This change requires a documentation update

How Has This Been Tested?

See above on "How to test the changes". Note that I have only tested this running in a Dev environment, using docker -- but I don't think that matters for this type of change.

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
  • Possibly a new log message is produced now.
• I have added tests that prove my fix is effective or that my feature works
  • I'm unclear how to proceed on this, but open to suggestions
• New and existing unit tests pass locally with my changes
  • Similarly, unclear on how to run these tests currently.
• [N/A] Any dependent changes have been merged and published in downstream modules

I wanted to initiate this Pull Request to get some formal code review here. This is my first time contributing on GitHub, so I'm not sure how this will go, but I would like to see review-comments on my existing changes, and I have a couple of followup questions I'd like to discuss before this is considered by me ready-to-merge. I'm hoping that can be done in the form of comments in the GitHub issues somehow -- but as this is my first, waiting to see how that works out.

[DawoudIO]

I been wanting to do this for a while let me test it and merge

[grayeul]

@DawoudIO - one of the questions I had is: If the CountryCode that is being used throughout the code is uppercase (e.g. US), then wouldn't it be better to change the locale/states/us.json file to be local/states/US.json, rather than force the name to lowercase here? I would think the filename should match the CountryCode, whatever the case is.
Hmm.. I'm not sure where it gets pulled in, but I now see ChurchCRM/Dashboard/CurrentLocaleMetadata.php is setting a 'countryFlagCode' to be the lowercase version of the CountryCode - so maybe the filename is appropriate, but I'm not sure if that should be referenced are used within this function somehow.

[grayeul]

Somewhat separate question -- but it didn't look like FamilyEditor.php was using the StateDropDown class (as PersonEditor is) -- but it seems to pull up the correct list -- so I'd like to understand how that is happening... but Also, when I tried to make a change to the State for an entry, using the FamilyEditor - I got this error:

PHP Fatal error:  Uncaught mysqli_sql_exception: Incorrect double value: 'NULL' for column `churchcrm`.`family_fam`.`fam_Latitude` at row 1 in /var/www/html/Include/Functions.php:193
Stack trace:
#0 /var/www/html/Include/Functions.php(193): mysqli_query()
#1 /var/www/html/FamilyEditor.php(337): RunQuery()
#2 {main}
  thrown in /var/www/html/Include/Functions.php on line 193

which I think is possibly due to FamilyEditor.php:1103 where I think "NULL" may be quoted and it shouldn't be? This appears to be a change about 3 weeks ago by @DAcodedBEAT , as part of "converting tedious sql strings to safer orm representation" - but I could be wrong. I did verify that removing the double-quotes around NULL solved the issue... I suppose I should open a new issue for that.

[DAcodedBEAT]

Somewhat separate question -- but it didn't look like FamilyEditor.php was using the StateDropDown class (as PersonEditor is) -- but it seems to pull up the correct list -- so I'd like to understand how that is happening... but Also, when I tried to make a change to the State for an entry, using the FamilyEditor - I got this error:

PHP Fatal error:  Uncaught mysqli_sql_exception: Incorrect double value: 'NULL' for column `churchcrm`.`family_fam`.`fam_Latitude` at row 1 in /var/www/html/Include/Functions.php:193
Stack trace:
#0 /var/www/html/Include/Functions.php(193): mysqli_query()
#1 /var/www/html/FamilyEditor.php(337): RunQuery()
#2 {main}
  thrown in /var/www/html/Include/Functions.php on line 193

which I think is possibly due to FamilyEditor.php:1103 where I think "NULL" may be quoted and it shouldn't be? This appears to be a change about 3 weeks ago by @DAcodedBEAT , as part of "converting tedious sql strings to safer orm representation" - but I could be wrong. I did verify that removing the double-quotes around NULL solved the issue... I suppose I should open a new issue for that.

Great find, I think you might be right.
I want to eventually update the codebase to use the ORM for updates and deletions as well (to avoid the string concatenation-to-build-queries mess we currently have), but since that will be a rather large change (like #6779 - converting tedious sql strings to safer orm representation was) it's probably a good idea to implement the short-term fix that you described in a separate pull request.

[grayeul]

Created issue #6833 to address the FamilyEditor problem. Will have an initial pull request for that soon.

[MrClever]

LGTM - I like what I see and it works in my test system

[romdricks]

Somewhat separate question -- but it didn't look like FamilyEditor.php was using the StateDropDown class (as PersonEditor is) -- but it seems to pull up the correct list -- so I'd like to understand how that is happening... but Also, when I tried to make a change to the State for an entry, using the FamilyEditor - I got this error:

Hi @grayeul
Did you figure out how FamilyEditor.php pull up the correct list?