Add fill options for Phone and E-mail details #83
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
|
I have also added a fix for #85 which deals with the same issue but then for email. It was not really possible to create two PRs because the code would interfere with this PR. |
artfulrobot
reviewed
Oct 17, 2021
| @@ -254,7 +254,18 @@ public function setLabel($label) { | |||
| * @throws Exception | |||
| */ | |||
| public function getOptions() { | |||
| return $this->getConfigGroup('options'); | |||
| $options = $this->getConfigGroup('options'); | |||
| // Backwards compatibility change fill details email to fill email. | |||
There was a problem hiding this comment.
Would this be better as an Upgradrer step?
| $location_type_id = $data['location_type_id']; | ||
| } | ||
|
|
||
| // Check whether a phone with this location type and phone type already exists. |
| if ($phone_type_id) { | ||
| $api_query['phone_type_id'] = $phone_type_id; | ||
| if ($fillOption == 3) { | ||
| $api_query['phone'] = $data[$attribute]; |
There was a problem hiding this comment.
I think this should use phone numeric so that different ‘spellings’ of the number match, e.g. 123 456 should be the same as 12 34 56.
jensschuppe
added
status:needs work
jensschuppe
changed the title
|
Hi @jaapjansma and @artfulrobot, we're currently compiling a |
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.
With the fill details options I would expect that a phone number is added when no phone number with the location type and phone type exists.
When a phone number with that location type and phone type already exists I would expect that nothing changes. However the new phone number gets added and I end up with two phones of the same location type and phone type.
This PR enhances the settings for fill details for phone numbers. The same issue also exists with e-mail addresses and this PR also fixes it for the e-mail addresses.
Before
Below a screenshot of the settings screen:
After
Below a screenshot with the changes applied to the settings screen:
And for e-mail
The changes are backwards compatible. Meaning that if someone had selected phone at the Fill details, the Fill Phone is set to
If contact has not this phone number.Comment
This PR also removes the phone_numeric field from the diff activity.