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#1930 fix for move-related checkbox being overridden to true … #18079

Merged
merged 1 commit into from
Aug 6, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Aug 6, 2020

Overview

Fixes a probable regression where deselecting 'move related contributions' etc does not block them being moved

ie activities are still transferred in this view
Screen Shot 2020-08-06 at 12 57 55 PM

See: https://lab.civicrm.org/dev/core/-/issues/1930

Before

Deselection ignored

After

Deselection works

Technical Details

I haven't managed to determine when this changed but the method of

$element->setChecked(TRUE);

is happening in preProcess and overwriting the submission - moving to setDefaults works

Comments

@civibot
Copy link

civibot bot commented Aug 6, 2020

(Standard links)

@civibot civibot bot added the master label Aug 6, 2020
@eileenmcnaughton eileenmcnaughton changed the base branch from master to 5.28 August 6, 2020 00:51
@civibot civibot bot added 5.28 and removed master labels Aug 6, 2020
@seamuslee001
Copy link
Contributor

(CiviCRM Review Template WORD-1.2)

  • General standards
    • (r-explain) Pass - Explained on the linked Ticket
    • (r-user) Pass
    • (r-doc) Pass
    • (r-run) Pass I replicated the problem on local dev by unchecking activities but the activities were still brought across in a merge, Reverting the database then applying this patch kept the activities with their original contacts but other elements were merged correctly
    • (r-tech) Pass
    • (r-code) Pass
    • (r-maint) Pass
    • (r-test) Pass

@seamuslee001 seamuslee001 merged commit 2a96c7c into civicrm:5.28 Aug 6, 2020
@seamuslee001 seamuslee001 deleted the 528 branch August 6, 2020 02:36
@pfigel
Copy link
Contributor

pfigel commented Aug 6, 2020

Thanks @eileenmcnaughton!

@eileenmcnaughton
Copy link
Contributor Author

thanks for finding it! I still don't fully get it since other fields the same didn't get the bug - but I'm gonna do another cleanup on it & see if I can get them all to use setDefaults - but in master

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.

3 participants