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#3050 - (Alternate) Fix crash removing an entry from a batch #24025

Merged
merged 1 commit into from
Jul 21, 2022

Conversation

demeritcowboy
Copy link
Contributor

Overview

https://lab.civicrm.org/dev/core/-/issues/3050

Alternate to #23957

Before

  1. Contributions - Accounting Batches - New batch
  2. Add one of the contributions to the batch.
  3. Using the "Remove" link on the far right of the row, remove the entry you just added.
  4. Crash

After

Ok

Technical Details

The backend was refactored in #21213 but it doesn't work here from the ajax call.

Comments

The test from the other PR won't work here. Not sure what to do for a test since it would mostly just be a test of calling api4.

@civibot
Copy link

civibot bot commented Jul 19, 2022

(Standard links)

@eileenmcnaughton
Copy link
Contributor

Ohh well done - yeah I was definitely in quote hell

@eileenmcnaughton
Copy link
Contributor

@demeritcowboy I tested this & it works so I'm going to merge it. There is a gap, however, in that I am pretty sure the permissions on the api are not well tuned and I think it falls back on administer CiviCRM but probably should be 'edit all manual batches' / 'edit own manual batches'

However - it's not working at all without this PR so the case to merge this in advance of any permission fix kinda makes itself :-)

@demeritcowboy
Copy link
Contributor Author

Thanks!

@demeritcowboy demeritcowboy deleted the batch-remove-2 branch July 21, 2022 11:45
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.

2 participants