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

Add MessageTemplate api to v4 #17073

Merged
merged 3 commits into from
Apr 17, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented Apr 14, 2020

Overview

Basic add of MessageTemplate entity to apiv4 & use if from core form

Before

Not there

After

Ta-da

Technical Details

I'm looking to cleanup this form to use the apiv4 (I'd rather got the extra step & switch it to an afform but
that seems like too big a leap).

This switches the loading to use apiv4. Note that

  1. I decided that it doesn't make sense to setCheckPermissions = FALSE - I think the form should not
    be availble to non-permissioned users (& perhaps a hook might like to play a role here).

  2. I removed the inheritence from the parent which seemed to do 3 things

  • added admin.css - none of the classes seemed to apply
  • added iconpicker - didn't seem to apply
  • loaded the defaults - which this change does on the form more succinctly

Comments

@civibot civibot bot added the master label Apr 14, 2020
@civibot
Copy link

civibot bot commented Apr 14, 2020

(Standard links)

@seamuslee001
Copy link
Contributor

@eileenmcnaughton i think that needs to be a subdirectory up i.e. in Civi/Api4

@eileenmcnaughton
Copy link
Contributor Author

opps you are right!

*
* @package CRM
* @copyright CiviCRM LLC https://civicrm.org/licensing
*/
Copy link
Member

Choose a reason for hiding this comment

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

RIP $Id$ we miss you.

@eileenmcnaughton
Copy link
Contributor Author

test this please

I'm looking to cleanup this form to use the apiv4 (I'd rather got the extra step & switch it to an afform but
that seems like too big a leap).

This switches the loading to use apiv4. Note that
1) I decided that it doesn't make sense to setCheckPermissions = FALSE - I think the form should not
be availble to non-permissioned users (& perhaps a hook might like to play a role here).

2) I removed the inheritence from the parent  which seemed to do 3 things
 - added admin.css - none of the classes seemed to apply
 - added iconpicker - didn't seem to apply
 - loaded the defaults - which this change does on the form more succinctly
@eileenmcnaughton
Copy link
Contributor Author

@colemanw can you recheck this - I altered the form to use apiv4 as well & added some notes to the PR template & I'd like your feedback as apiv4 calls in core are fairly new

In the process of this I would note that I could also have used apiv3 & I can now see that the EntityFormTrait would have worked too (FYI ) but I was kind of trying to get familiar with increasing v4 usage in core

@mattwire we should possibly switch the EntityFormTrait to use apiv4. I think we could just switch

    $metadata = civicrm_api3($this->getDefaultEntity(), 'getfields', ['action' => 'create']);

to

    $metadata = civicrm_api4($this->getDefaultEntity(), 'getfields', ['action' => 'create']);

But we'd need some noisy fallback to apiv3 where there isn't a v4 entity

@mattwire mattwire changed the title Add MsgTemplate api to v4 Add MessageTemplate api to v4 Apr 15, 2020
@seamuslee001 seamuslee001 merged commit 9a0c2cd into civicrm:master Apr 17, 2020
@seamuslee001 seamuslee001 deleted the msg_template branch April 17, 2020 02:00
@colemanw
Copy link
Member

Yes it all still looks good.

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