Country field added for international schools

[razztech]

Pivotal Tracker Link

What this PR does:

This PR adds the country field to the school model. Whenever a new school is created, the option to select a country is available. Moreover, the country data is introduced in tables, teacher and school information panels and in the backend. The UI logic either renders a textfield if the country selected when creating a school is not the US or a dropdown of US states if the selected country is the US.

Include screenshots

Who authored this PR?

• @razztech overall implementation, testing, code refactoring and UI logic
• @perryzjc pair-programming help for debugging teacher requests table

How should this PR be tested?

1. Create a new request as a teacher
2. Input a school that is not present in the dropdown and click enter
3. Select the country of your choice outside the US and type a random state in the State field
4. Submit your request.
5. Login as an admin and see that the country selected and the state are present in the data.

What do the specs/features test?

• The feature tests if there is a country outside of the US selected then we should be able to type in the name of the state for that country instead of selecting from the usual US states dropdown.

Are there edge cases to watch out for?

No.

Migration

• 20240307225738_add_country_to_schools.rb = adds the country field to the school model;
• Should use rails db:migrate to update the model

New Gems

• Installed the country_select gem for the country dropdown that includes the countries and unaccent gems
• Need to run bundle install to use these gems

Checklist:

• Has this been deployed to a staging environment or reviewed by a customer?
• Tag someone for code review (either a coach / team member)
• I have renamed the branch to match PivotTracker's suggested one (necessary for BlueJay) (e.g. michael/12345-add-new-feature Any branch name will do as long as the story ID is there. You can use git checkout -b [new-branch-name])

[codecov]

Codecov Report

Attention: Patch coverage is 90.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 86.18%. Comparing base (5135d22) to head (da5f122).

Files	Patch %	Lines
app/controllers/schools_controller.rb	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #42      +/-   ##
==========================================
- Coverage   86.29%   86.18%   -0.11%     
==========================================
  Files          20       20              
  Lines         693      695       +2     
==========================================
+ Hits          598      599       +1     
- Misses         95       96       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[perryzjc]

@razztech Nice job! This is a significant amount of work. I have a few notes for this PR:

1. Based on the code review Michael provided during today's lecture (Mar 11), he mentioned that it might be better not to include customized steps within web_steps.rb. Instead, it's recommended to place them in a separate customized steps file. form_steps.rb might be the ideal one for the case for this PR.

2. Depending on Michael's preferences, it might be more comprehensive to include tests like I should see "AL" to verify the existence of the new country field for both the teacher and school tabs.

3. This PR could benefit from more detailed documentation:

   • You already mentioned 20240307225738_add_country_to_schools.rb = adds the country field to the school model; but it might be clearer to explicitly mention running rails db:migrate for it.
   • Additionally, since this PR introduces new Gem files, it would be helpful to include instructions to run bundle install.

4. Some clarifications might be needed from Michael:

   • When the country field is displayed, do we want it in abbreviated form or in full form? For example, should it be 'China' or 'CN'? The current version is only displaying the abbreviated form, such as AL and RO, which might not be very readable:

   

[fp456]

Good job! It seems very good overall 🙌 just some tiny things here and there regarding testing and some clean up.

[perryzjc]

New notes for today's customer meeting with Michael:

1. Put LOCATION AND COUNTRY TOGETHER.
2. School name should take up the entire row on the screen (on the new school page).
3. City, state, country (in that order) should occupy the entire row on the screen (on the new school page).
4. Default to US.

[razztech]

[razztech]

[ArushC]

@razztech LGTM. Just for future reference, I find it helpful to use the $(...) jQuery notation when writing Javascript. I asked ChatGPT to rewrite your JS code using $(...) stuff and see below for what it came up with. Idk if it works, but if you want you can test it. Personally, I think it's a lot easier to read. I'm going to wait for someone else to review changes before merging because I'm not 100% confident I understand everything that's going on here. @perryzjc you should probably review.

<script>
  $(document).ready(function() {
    const countrySelected = $('#school_country');
    const stateSelectContainer = $('#state_select_container');
    const stateTextfieldContainer = $('#state_textfield_container');
    const stateSelect = $('#state_select');
    const stateTextfield = $('#state_textfield');

    function handleCountryChange() {
      if (countrySelected.val() === 'US') {
        stateSelectContainer.show().attr('required', true);
        stateTextfieldContainer.hide();

        stateTextfield.removeAttr('name'); 
        stateSelect.attr('name', 'school[state]');
      } else {
        stateTextfieldContainer.show().attr('required', false);
        stateSelectContainer.hide();

        stateSelect.removeAttr('name');
        stateTextfield.attr('name', 'school[state]');
      }
    }
    countrySelected.change(handleCountryChange);
    handleCountryChange(); 
  });
</script>

[perryzjc]

@razztech LGTM. Just for future reference, I find it helpful to use the $(...) jQuery notation when writing Javascript. I asked ChatGPT to rewrite your JS code using $(...) stuff and see below for what it came up with. Idk if it works, but if you want you can test it. Personally, I think it's a lot easier to read. I'm going to wait for someone else to review changes before merging because I'm not 100% confident I understand everything that's going on here. @perryzjc you should probably review.

<script>
  $(document).ready(function() {
    const countrySelected = $('#school_country');
    const stateSelectContainer = $('#state_select_container');
    const stateTextfieldContainer = $('#state_textfield_container');
    const stateSelect = $('#state_select');
    const stateTextfield = $('#state_textfield');

    function handleCountryChange() {
      if (countrySelected.val() === 'US') {
        stateSelectContainer.show().attr('required', true);
        stateTextfieldContainer.hide();

        stateTextfield.removeAttr('name'); 
        stateSelect.attr('name', 'school[state]');
      } else {
        stateTextfieldContainer.show().attr('required', false);
        stateSelectContainer.hide();

        stateSelect.removeAttr('name');
        stateTextfield.attr('name', 'school[state]');
      }
    }
    countrySelected.change(handleCountryChange);
    handleCountryChange(); 
  });
</script>

Just finished review. Looks to me as well, and I helped fix the reviews I came up, plus adding a new test for undesired user behavior

[razztech]

Thank you @perryzjc!