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

dev/core#692 Unable to use url search arguments in 'Advanced Search' using force=1 #14418

Closed
wants to merge 2 commits into from

Conversation

monishdeb
Copy link
Member

@monishdeb monishdeb commented Jun 3, 2019

Overview

Currently, it is not possible to use URL search arguments in 'Advanced Search' using force=1 to load search results. This is no longer working except for mailing params which are manually handled here

https://lab.civicrm.org/dev/core/issues/692

Before

You cannot use any search parameters as url arguments to load search results in 'Advance Search'.

After

You cannot use contact search parameters as url arguments to load search results in 'Advanced Search'. Here are two examples:

  1. Using contact_type=Individual
    before

  2. Log date = true AND log_date_relative = This calendar month
    after

  3. Privacy options - do_not_email + privacy operator - AND + privacy toggle - Include
    after

Technical Details

This fix is made for Contact search fields and can be extended for other component's search forms.
Please check the issue description here for more details.

Comments

ping @lcdservices @eileenmcnaughton @seamuslee001

@civibot
Copy link

civibot bot commented Jun 3, 2019

(Standard links)

@civibot civibot bot added the master label Jun 3, 2019
@lcdservices
Copy link
Contributor

I ran through a number of tests and this works as expected.

@colemanw
Copy link
Member

colemanw commented Jun 3, 2019

@monishdeb test failures may be related.
@civicrm-builder retest this please

@monishdeb
Copy link
Member Author

@colemanw seems like test build is triggered again somehow.

@colemanw
Copy link
Member

colemanw commented Jun 3, 2019

@monishdeb I triggered it with my comment. Here are the previous results: https://test.civicrm.org/job/CiviCRM-Core-PR/26673/

@monishdeb monishdeb force-pushed the core-692 branch 2 times, most recently from 5f73244 to 5338ebf Compare June 5, 2019 14:06
eileenmcnaughton added a commit to eileenmcnaughton/civicrm-core that referenced this pull request Jun 7, 2019
@eileenmcnaughton
Copy link
Contributor

@monishdeb my worry is that this is using metadata but it's pushing in a different direction & using different constructs than what we have been doing on other forms. I just converted one field using our methodologies from the other forms - #14475 - I also have a thought how better to combine with the existing getBasicSearchFields fn but I figure if we can extract that first it will be clearer - so I put in #14476

@eileenmcnaughton
Copy link
Contributor

@monishdeb so this picks off a couple more fields in a way that is consistent with work on other forms

https://github.com/civicrm/civicrm-core/pull/14921/files

Note the metadata drives the field being added to the form, grouped on the template and the loading of defaults

@eileenmcnaughton
Copy link
Contributor

@monishdeb I feel like we superceded this one...

Once we fork for 5.17 we will probably convert some more datepicker fields which should implicitly add them to the url-supported fields.

I'd rather do a small number of fields at a time & deal with the tech issues each one throws up (if any)

@mlutfy mlutfy changed the title Core/dev#692 : Unable to use url search arguments in 'Advanced Search' using force=1 dev/core#692 Unable to use url search arguments in 'Advanced Search' using force=1 Nov 22, 2019
@mlutfy
Copy link
Member

mlutfy commented Nov 22, 2019

Hi Monish,

I will close this PR for now because it is stale, but feel free to re-open when it's ready for a new round of review.

@mlutfy mlutfy closed this Nov 22, 2019
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.

5 participants