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

SearchKit - Make inline edit only available when applicable #23404

Merged
merged 2 commits into from
May 21, 2022

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented May 9, 2022

Overview

Fixes dev/core#3423 Make inline edit for custom fields only available when applicable

Before

Custom fields presented as editable for entity sub-types that they were not applicable to, e.g. if a set of custom fields only belongs to Activities of type Meeting, it would be editable for all activities in the SearchKit results table.

After

Correctly shown as editable only when applicable.

@civibot
Copy link

civibot bot commented May 9, 2022

(Standard links)

@civibot civibot bot added the master label May 9, 2022
@seamuslee001
Copy link
Contributor

@colemanw error

Fatal error: Cannot declare class api\v4\Custom\BasicCustomFieldTest, because the name is already in use in /home/jenkins/bknix-dfl/build/core-23404-23zzz/web/sites/all/modules/civicrm/tests/phpunit/api/v4/Custom/BasicCustomFieldTest.php on line 34

@colemanw colemanw force-pushed the getFieldsFilters branch from d1ab3e6 to 82ddd53 Compare May 9, 2022 13:49
@colemanw
Copy link
Member Author

colemanw commented May 9, 2022

retest this please

@colemanw
Copy link
Member Author

This PR triggered a seemingly-unrelated-yet-persistent test fail which took me down a rabbit-hole to refactor the entire APIv4 test suite:
#23432

@colemanw colemanw force-pushed the getFieldsFilters branch from 82ddd53 to afc0907 Compare May 11, 2022 15:34
@eileenmcnaughton
Copy link
Contributor

@colemanw needs rebase

@colemanw colemanw force-pushed the getFieldsFilters branch 2 times, most recently from f5f79a6 to 8970e37 Compare May 19, 2022 00:06
@colemanw
Copy link
Member Author

@francescbassas thanks for your patience. Can you please review this PR once more so we can get it merged.

While testing this, I found a pre-existing bug which I'll fix in a separate PR: editing a custom relationship field on a related contact search now "works" as in the field is only editable if applicable to that relationship type. However, the inline-edit results in the wrong relationship being updated! Please ignore that issue when testing this PR as it's pre-existing and will be addressed separately.

@colemanw colemanw force-pushed the getFieldsFilters branch from 8970e37 to 78b46e0 Compare May 19, 2022 01:06
@colemanw
Copy link
Member Author

Wrong-record-edit bug fixed in #23496

@francescbassas
Copy link
Contributor

@colemanw thanks for your work!

Test site is down? http://core-23404-2mlpw.test-3.civicrm.org:8001

Is there any way to up it so that I can test it directly there?

@colemanw
Copy link
Member Author

Not sure what happened to that site but @civicrm-builder retest this please.
That will generate a new site (come back in about an hour)

@colemanw
Copy link
Member Author

colemanw added 2 commits May 20, 2022 21:30
Fixes dev/core#3423
Ensures that custom fields for one entity sub-type (e.g. Individual) are not
presented as editable for other sub-types (e.g. Organiztion).
@colemanw colemanw force-pushed the getFieldsFilters branch from 78b46e0 to 489257b Compare May 21, 2022 01:30
@colemanw
Copy link
Member Author

Merging based on review from dev/core#3423

@colemanw colemanw merged commit be5972c into civicrm:master May 21, 2022
@colemanw colemanw deleted the getFieldsFilters branch May 21, 2022 13:52
colemanw added a commit to colemanw/civicrm-core that referenced this pull request Aug 14, 2022
colemanw added a commit to colemanw/civicrm-core that referenced this pull request Aug 14, 2022
eileenmcnaughton pushed a commit to eileenmcnaughton/civicrm-core that referenced this pull request Aug 23, 2022
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.

4 participants