dev/core#1895 fix first/last name adv search #17950
Conversation
|
(Standard links)
|
|
This patch exposes a bug in metadata function where it shows that if we explicitly declare a form element in a search form, it works and also retain the keyword on form reload. But when we use the metadata function to add search fields first and last name it doesn't work. Reason why in the patch, @lcdservices has tried to move the fields declaration from metadata to search form. I am trying to figure out the cause why its only happening with these two fields not with sort_name or other basic contact fields. |
|
@lcdservices @seamuslee001 I have made the fix and updated the PR. First and last name is now working on advance search and also with quick search. Please have a look. |
|
This is working. I'm comfortable with the code. It approaches the issue in a slightly different way than I had proposed, but addresses the same issues I had run into. |
|
test this please |
|
This test is very form layer so I don't think it needs a test. I'm comfortable with the approach and the face @lcdservices has r-run it. My one question preventing me merging this now is that I wonder if it shouldn't go against the rc. Based on that I'm going to add merge-ready. One way or another we should merge before the next rc is cut |
eileenmcnaughton
added
the
merge ready
Test Result (1 failure / +1)api_v3_JobTest.testNoErrorOnOddpresumably unrelated but not familiar |
|
test this please |
|
@lcdservices @monishdeb same error - I think it must relate |
|
Looking at these test failure |
monishdeb
force-pushed
the
dev-core-1895
branch
from
9fe01c4 to
b54a867
Compare
| @@ -10,7 +10,7 @@ | |||
| <div class="advanced-search-fields basic-fields form-layout"> | |||
| {foreach from=$basicSearchFields item=fieldSpec} | |||
| {assign var=field value=$form[$fieldSpec.name]} | |||
| {if $field} | |||
| {if $field && !in_array($fieldSpec.name, array('first_name', 'last_name'))} | |||
There was a problem hiding this comment.
Do we suppress the fields here?
There was a problem hiding this comment.
@eileenmcnaughton no. As in sort_name.tpl we are already rendering these two fields, reason why we are preventing first_name and last_name to be added as a separate field in advance search form.
|
@seamuslee001 I know you were digging into this - it looks OK to me - if it does to you please merge |
|
@seamuslee001 we should put against the rc now? Where is this at from your pov? |
seamuslee001
added
merge on pass
and removed
merge ready
|
I'm good with this updated the base branch and changed to MOP |
|
Thanks @eileenmcnaughton @seamuslee001 |
Overview
The ability to search first/last name was recently added to core. The primary intent was to properly handle advanced searches that originate from quicksearch first/last name filters. While that was working, a first/last name search that originated directly from an advanced search was not working. This fixes the issue.
Before
Go to advanced search, expose the first/last name fields, enter criteria and initiate the search. Result = all records returned and the filter value was not preserved.
After
Conduct search and it returns results as expected.
Technical Details
There were a couple issues with the original patch that needed to be addressed. The first/last name fields needed to be constructed in the form explicitly, rather than through the metadata function, as they were not handled properly by the form via the previous method. And the js that toggles between the sort_name and first_name/last_name fields needed to use a method other than show()/hide() as the resulting display:none meant that the form values were lost on reload.