Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PHP8 warning on DedupeFind #26377

Merged

Conversation

larssandergreen
Copy link
Contributor

@larssandergreen larssandergreen commented May 30, 2023

Overview

When group id isn't passed to DedupeFind, we get a warning.

Passing gid=0 or NULL doesn't fix the error, there must be some code that doesn't set the variable if it isn't truthy.

@civibot
Copy link

civibot bot commented May 30, 2023

(Standard links)

@civibot civibot bot added the master label May 30, 2023
@demeritcowboy
Copy link
Contributor

The only problem is this won't be compatible with CIVICRM_SMARTY_DEFAULT_ESCAPE. Which reminds me I need to do something with this.

@larssandergreen
Copy link
Contributor Author

@demeritcowboy Right, I forgot about that one.

I was looking at Form/DedupeFind.php and I should have been looking at Page/DedupeFind.php, now things make more sense, have pushed a new version that fixes this properly.

Yes, would be great to add those details to the docs. It's hard to find on Gitlab.

@larssandergreen larssandergreen force-pushed the Fix-PHP8-warning-on-DedupeFind branch from ce4342e to 6898380 Compare May 30, 2023 14:44
@demeritcowboy
Copy link
Contributor

What is gid? I can't see anywhere it gets passed in, but even if it did, with regard to this notice, all it does is pass it on in the ajax call to toggleDedupeSelect, which calls CRM_Contact_Page_AJAX::toggleDedupeSelect, which doesn't use it.

Maybe this is one of those where the notice is trying to tell us something?

@larssandergreen larssandergreen force-pushed the Fix-PHP8-warning-on-DedupeFind branch from 6898380 to 7a7ecd2 Compare May 30, 2023 16:38
@larssandergreen
Copy link
Contributor Author

gid is the group id for limiting matches to a group. It is passed in from the URL.

But you are right that it (and in fact rgid as well) are not needed by toggleDedupeSelect, so I think we can cut them. Pushed a new version that does that.

I don't know what snippet=4 is doing there, so I'm not going to touch it.

@demeritcowboy demeritcowboy merged commit e742dd4 into civicrm:master May 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants