Remove calls & deprecate CRM_Core_BAO_PrevNextCache::setItem #16696
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)
|
We have this function which 1) is mildly misleading - it appears to be generic to the prevnext table but is actually only relevant to deduping as searches no longer use it 2) is about 50% deprecated and 3) the remainder is a single insert 4) is called from 2 places which use it a little differently I think it's not really adding much value - I was going to remove the deprecated code but I think in fact the goal should be to remove the whole function. In general I think code on CRM_Core_BAO_PrevNextCache that is really dedupe-only code should be on the dedupe classes. The history is that prevnext was created for searches and kindof twisted to support dedupe as well but now search doesn't use much of what is in the BAO class (if any)
|
test this please |
|
@eileenmcnaughton i'm generally ok with the direction of this but i still feel like having a central function that does the insert/update makes sense to me, But if that means its on the Dedupe class then fine tbh |
|
@seamuslee001 I think 'parseAndStoreDupePairs' is a reasonable function name for doing this - which is the calling function. The only other place is kind of a hack so not sharing & eventually moving towards a better plan makes sense |
|
@seamuslee001 passing now |
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
Remove calls & deprecate CRM_Core_BAO_PrevNextCache::setItem
Before
Remove calls & deprecate CRM_Core_BAO_PrevNextCache::setItem Function called twice - but trivially - no reduction in code lines or readability improvement from doing so
After
Remove calls & deprecate CRM_Core_BAO_PrevNextCache::setItem unused & deprecated
Technical Details
We have this function which
only relevant to deduping as searches no longer use it
I think it's not really adding much value - I was going to remove the deprecated code
but I think in fact the goal should be to remove the whole function.
In general I think code on CRM_Core_BAO_PrevNextCache that is really dedupe-only code
should be on the dedupe classes. The history is that prevnext was created for searches
and kindof twisted to support dedupe as well but now search doesn't use much of what
is in the BAO class (if any)
Comments