PCHR-4346: CiviCRM Upgrade #2899
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
davialexandre
force-pushed
the
PCHR-4346-civicrm-upgrade
branch
from
6be9ed0 to
b728232
Compare
This was referenced Nov 1, 2018
…FullDetails API This API call is an optimized way to fetch all the information for a Job Contract, which is stored in multiple different database tables. It does that by running a single SQL query, joining all the tables and the necessary fields. In the resultset, in order to differentiate which field belongs to which entity, each field is prefixed with the entity name (examplei: details__<field_name>). After the data is fetched from the database, the `HRJobContractRevision::normalizeFullDetailsResult()` was used to loop through all the fields in the resultset, detect to which entity they belong to and organize the data in a proper way. Besides the fields returned by the query, the resultset object also contains some internal fields. In the past, all of these fields were prefixed with an underscore, but on civicrm/civicrm-core#12276 a new `resultCopies` field was added, and since it does not start with an underscore, the logic to filter out the internal fields stopped working. To fix that, instead of looping through all of the fields from the resultset, we call the `toArray()` method which will return a list of fields and values containing only those returned by the SQL query.
In civicrm/civicrm-core@4aced66, the Summary.tpl file has been refactored in core and part of it was extracted to the Basic.tpl template. Our customization was located exactly in the piece of code that has been extracted, so the sync was done by: - Overridding the Basic.tpl template in hrui - Moving the custom code from the Summary.tpl file to the new Basic.tpl one - Deleting the old Summary.tpl file, as it didn't contain any custom code anymore As part of this, I also did some small refactoring in the custom code, removing some unecessary ifs and custom variables to make it less different from the original file as possible
The only reason we needed the overridden file was to fix an error in CiviCRM Core. Since this has been now fixed in core, we don't need to file anymore. References: civicrm/civicrm-core#13031 compucorp/civicrm-core#28
davialexandre
force-pushed
the
PCHR-4346-civicrm-upgrade
branch
from
d87a2af to
0232ce4
Compare
davialexandre
changed the title
tunbola
approved these changes
Nov 7, 2018
Now that the final version has been released, we don't need to use the branch version anymore
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This PR updates CiviHR to work with CiviCRM 5.7.0.
Technical Details
Syncing overridden files
As usual, overridden files have been synced with core. Here is the list of them with a link to a diff comparing them to the respective file in core. Note that not all the overridden files required updates, as some their originals in core have not been touched since CiviCRM 5.3.1:
CRM/Hrjobcontract/Dedupe/Merger.php: difftemplates/CRM/Hrjobcontract/Form/Merge.tpl: diffhrjobcontracts/CRM/Export/BAO/Export.php: diffThe
hrui/templates/CRM/Contact/Page/View/Summary.tploverridden file has been replaced with thehrui/templates/CRM/Contact/Page/Inline/Basic.tplfile. The reason for this is this PR where part of the template code was extracted to another file. Our customized code was exactly in the part that was extracted, so now we override the new extracted file instead. Here is a diff showing the customizations: https://www.diffchecker.com/tDtrMUUaBesides that, I've taken the chance to optimize the customized code in that file by removing some duplication and unecessary variables. You can see the difference by comparing the previous diff with the one for the
Summary.tplfile from the previous CiviCRM upgrade: https://www.diffchecker.com/c0rENpFoThe
hrui/CRM/Core/BAO/Navigation.phpoverridden file has been deleted. This file has been overridden for multiple different reasons in the past, but today, it was holding only one customization to fix a bug in CiviCRM core. So, instead of syncing the file, I decided to submit a PR fixing this in core. It will be only included in CiviCRM 5.8, so I've added it to your patches branch.Undefined index error in the HRJobContract.getFullDetails API
After the upgrade, the tests for this API started failing. This was caused by a new property added to the DAO class in CiviCRM. Check the commit message of b728232 for more details