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#2486 Add v4 batch api #19931

Merged
merged 1 commit into from
Mar 29, 2021
Merged

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

dev/core#2486 Add v4 batch api

Before

the abyss

After

tada

Technical Details

Comments

@civibot
Copy link

civibot bot commented Mar 28, 2021

(Standard links)

@civibot civibot bot added the master label Mar 28, 2021
@@ -17,7 +17,6 @@
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eileenmcnaughton why are you moving this file? i'm not sure if it will then be picked up because the class name doesn't match the file name then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ug that was an opps / ide trying to fix up psr issues

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eileenmcnaughton
Copy link
Contributor Author

This failed because 'title' is not required but 'kinda is' in the BAO

I made it required - but I haven't done the upgrade script as it would conflict with #19512

In addition I'm leaning around 80% towards using a hash as a default for title in the BAO since I feel like batches don't super need meaningful titles

What do you think @seamuslee001 @colemanw - this is the line

$params['name'] = CRM_Utils_String::titleToVar($params['title']);

@colemanw
Copy link
Member

I think supplying a random string as name is fine if ommitted.

@eileenmcnaughton
Copy link
Contributor Author

@colemanw OK it will now do 'batch_ref' + a random number

@eileenmcnaughton eileenmcnaughton merged commit 177b66d into civicrm:master Mar 29, 2021
@eileenmcnaughton eileenmcnaughton deleted the fa branch March 29, 2021 19:09
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