feat: add country support#742
Conversation
87e5e60 to
261c9ee
Compare
|
Please merge #743 first. In order to keep the country fixture semantically correct, I changed
to
Without #743, this branch still needs a workaround in |
|
My main concern here is that he google geocoding in /lib/ZipLookup.php is not broken. Google lookup currently returns the country long name, while the new country select appears to expect the ISO-style country code. Could you please make sure the zip/geocode lookup is also amended so it maps Google’s country component using short_name rather than long_name, and stores the values like "US" rather than "United States"? |
261c9ee to
5b6a67c
Compare
5b6a67c to
9d31035
Compare
618c0cb to
5c80c52
Compare
RussH
left a comment
There was a problem hiding this comment.
One thing I’d like checked before merge: please re-check the final argument ordering in the CareersUI::onApplyToJobOrder() calls to Candidates::add() and Candidates::update().
Candidates::add() now has $skipHistory = false, $country = '' at the end, while Candidates::update() now has $country = false at the end. The careers portal calls appear to add $country in a way that may not line up with those signatures, especially the update call where an extra false is passed before $country. That could cause issues?
Minor maintainability point: there are also a few new direct htmlspecialchars() calls when building the country dropdown. These should ideally reuse the existing OpenCATS escaping/helper approach rather than introducing more inline escaping logic.
If you can fix the candidate add/update argument ordering and fix the merge conflict I’m happy with the rest of the direction.
5c80c52 to
cd1c227
Compare
cd1c227 to
4499cce
Compare
* Remove legacy `<city>` / `<state>` placeholder handling from the public Career Portal template rendering and rely on `<location>` only * Update the shipped default Job Details template to use `<location>` and add a safe schema migration that upgrades stored default Job Details templates * Note: Custom Career Portal templates that still reference `<city>` and/or `<state>` will no longer render a location and must be updated to use `<location>`
4499cce to
11f2d69
Compare
RussH
left a comment
There was a problem hiding this comment.
One thing to double-check?
Can you re-check the argument order for the candidate add/update calls, especially in the career portal apply/update flow? Since Candidates::add() and Candidates::update() have slightly different trailing args, I want to make sure $country is not getting passed into the wrong slot or skipped in one path.
For the country dropdown output, can we use the existing OpenCATS functions instead of direct htmlspecialchars() calls? For normal HTML text output, Template::escapeHtml($value) is the helper I’d expect?
|
Thanks @RussH. I believe this should already be addressed in the follow-up changes from last month: https://github.com/opencats/OpenCATS/compare/5c80c52ed7411a4331cd2320aaa5241188031c75..cd1c227d2785515aaeeb87994f8d324e6f269670 I re-checked the current branch and the career portal calls now pass the candidate country in the expected position. For The country dropdown is also generated through That is why I requested another re-review last month after pushing those changes. Please let me know if I missed a remaining case or if you had a different spot in mind. |
Summary
This PR adds country support to the core address-bearing entities in OpenCATS by introducing a
countryfield for companies, contacts, candidates and job orders across the schema, legacy data layer, UI forms, AJAX location helpers and the newer entity/repository coverage where applicable.It also adds country selection support to the Career Portal, including the shipped default apply template and candidate profile handling, so candidate address data can be captured and preserved consistently instead of assuming a single-country setup.
In addition, this PR updates Career Portal job location rendering to use a unified
<location>placeholder so locations can include country information cleanly. The default Job Details templates are updated accordingly, and a schema migration upgrades stored default templates that still use the legacy<city>/<state>pattern.Note: Custom Career Portal templates that still reference
<city>and/or<state>will no longer render a location and must be updated to use<location>Motivation
The main goal of this change is to make OpenCATS usable for recruiters and staffing agencies that operate across national borders by introducing an explicit way to distinguish and store country information.
It also serves as groundwork for restoring zip lookup on top of country-aware location data, so future improvements can avoid relying on a fixed country assumption such as the current US-based behavior.