Don't throw PHP notice when rendering dedupefind template. #22709
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
|
(Standard links)
|
colemanw
reviewed
Feb 5, 2022
| if (Civi::settings()->get('dedupe_default_limit')) { | ||
| $this->add('text', 'limit', ts('No of contacts to find matches for ')); | ||
| } | ||
| $this->add('text', 'limit', ts('No of contacts to find matches for ')); |
There was a problem hiding this comment.
It's a pre-existing condition, but this text makes no sense to me, and trailing spaces are not allowed within ts()
There was a problem hiding this comment.
@colemanw I do agree, but I'm struggling to think of a better string (other than writing "number" in full). Maybe a {help} popup is required to add some longer-form description?
|
I've stepped through the code and confirm this is the same before/after, just without the php notice. |
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
Don't throw PHP notice when rendering dedupefind template.
Before
A number of notices occured from the Smarty template when finding duplicate contacts:
These only occur if no default dedupe limit
dedupe_default_limitis setAfter
The notices related to the limit field no longer occur. (Note there is still a
Undefined index: gid, which appears to be coming from a slightly different template).Technical Details
I was slightly confused by the logic around when the "No of contacts to find matches for" should be shown. It appears that it is only shown if a default limit is set, presumably in order to avoid showing unnecessary fields to administrators of smaller networks.
The visibility is now controlled by a new template variable
limitShown, which uses the same logic as is currently in place. One benefit of this change is that it is now possible for site owners to ensure the limit field is shown or hidden regardless of the global setting, by setting thelimitShownvariable withinhook_civicrm_buildForm.