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

Enable Case search metadata on Advanced Search Form #15928

Merged
merged 1 commit into from
Nov 25, 2019

Conversation

seamuslee001
Copy link
Contributor

Overview

Enables urls like civicrm/contact/search/advanced?reset=1&force=1&case_subject=test to work

Before

Case parameters don't work on Advanced Search

After

Do work

ping @eileenmcnaughton @demeritcowboy

One thing tho the case_subject one does generate the notice htmlspecialchars() expects parameter 1 to be string, array given in HTML_Common->_getAttrString() (line 144 of /home/seamus/buildkit/build/47-test/sites/all/modules/civicrm/packages/HTML/Common.php). but only on Advanced Search with this PR.

@civibot
Copy link

civibot bot commented Nov 22, 2019

(Standard links)

@civibot civibot bot added the master label Nov 22, 2019
@demeritcowboy
Copy link
Contributor

It works just:

  • Can confirm there's the mentioned warning above.
  • On the Find Cases page you can use sort_name=. That doesn't seem to work here and just shows all contacts. It does fill in the form field though.
    • Looks like it's like this on dmaster too, so I don't know if that ever worked for advanced search.

Interesting sidenote: I didn't think it would work but it DOES work to specify component_mode via url to show results as cases, but you need to know that the code is 2048. But there's other things like that too, e.g. case_type_id.

@seamuslee001
Copy link
Contributor Author

@demeritcowboy @eileenmcnaughton i have just pushed a new commit which fixes the e-notice error but would appreciate especially Eileen's thinking on whether it is appropriate fix or not

@seamuslee001 seamuslee001 force-pushed the case_advanced_search branch 2 times, most recently from bc078e5 to 4fa1f51 Compare November 23, 2019 03:32
@eileenmcnaughton
Copy link
Contributor

@seamuslee001 so the ideal would be to remove that line - ie

 $this->set('formValues', $this->_formValues);

CRM_Contact_Form_Search_Advanced called $this->setFormValues();
at the start & I think that's the right place.

The issues are

  1. other forms extend the CRM_Contact_Form_Search class &
  2. I guess there is some chance that something that has changed in the in-between lines matters (although I suspect it just adds complexity).

I suspect we could mitigate the latter by removing the if (!empty($_POST) from the call to normalize form values & the former by having it still call $this-setFormValues where it currently does a

$this->set('formValues', $this->_formValues);

but within formValues have some mechanism so it only works once - ie. if (!$this->isSetFormValues....) & then set it at the end.

@seamuslee001
Copy link
Contributor Author

ok @eileenmcnaughton i have pulled out that 2nd commit now so i think this is more mergable now

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 - I'm OK to merge this as I think we are already in a situation where we need to handle the LIKE e-notice before the rc is cut & we've started on that. This doesn't exacerbate that by much

@monishdeb
Copy link
Member

Merging as per tag.

@monishdeb monishdeb merged commit af8faeb into civicrm:master Nov 25, 2019
@eileenmcnaughton eileenmcnaughton deleted the case_advanced_search branch November 25, 2019 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants