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

afform - add dynamic forms for creating/update custom group data #31484

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ufundo
Copy link
Contributor

@ufundo ufundo commented Nov 15, 2024

Overview

Currently afform blocks are auto generated for custom group data, which you can add to your own forms.

This adds automatic forms which can be used to edit the custom group data.

Comments

I've moved the form generation from the Afform/Get class to a dedicated action on CustomGroup entity, as it gets a bit more involved creating multiple forms for each Group.

Still a bit WIP.

Questions:

  • For multi record custom groups we have to wrangle with af-join - I think it could be nice if we could enable them to be the primary form entity (with a required entity_id field).
  • Currently I've implemented this directly in afform core - because that's where the blocks generation was, and it's nice to keep it together - but I do wonder if the forms should be in civicrm_admin_ui, because there are a bit more involved
  • Permissions: I think the forms should be safe as long at they use RBAC - but not sure exactly what the (default) form level permission should be

Next step:

  • Search forms to display multi value data. This requires creating SearchKits as well.

Copy link

civibot bot commented Nov 15, 2024

🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷

Introduction for new contributors...
  • If this is your first PR, an admin will greenlight automated testing with the command ok to test or add to whitelist.
  • A series of tests will automatically run. You can see the results at the bottom of this page (if there are any problems, it will include a link to see what went wrong).
  • A demo site will be built where anyone can try out a version of CiviCRM that includes your changes.
  • If this process needs to be repeated, an admin will issue the command test this please to rerun tests and build a new demo site.
  • Before this PR can be merged, it needs to be reviewed. Please keep in mind that reviewers are volunteers, and their response time can vary from a few hours to a few weeks depending on their availability and their knowledge of this particular part of CiviCRM.
  • A great way to speed up this process is to "trade reviews" with someone - find an open PR that you feel able to review, and leave a comment like "I'm reviewing this now, could you please review mine?" (include a link to yours). You don't have to wait for a response to get started (and you don't have to stop at one!) the more you review, the faster this process goes for everyone 😄
  • To ensure that you are credited properly in the final release notes, please add yourself to contributor-key.yml
  • For more information about contributing, see CONTRIBUTING.md.
Quick links for reviewers...

➡️ Online demo of this PR 🔗

@civibot civibot bot added the master label Nov 15, 2024
@ufundo
Copy link
Contributor Author

ufundo commented Nov 15, 2024

@colemanw just putting this up as WIP on the custom group afforms. Started on the create/update forms as they are a bit simpler, but have made some in-roads into the search listings too.

Got a bit bogged down in hook_civicrm_tabset and the afform implementation thereof though.

I wondered about just using an afform with a url to add the tabs directly, rather than via the afform placement tag?

e.g. the following basically seemed to work

/**
 * Implements hook_civicrm_tabset().
 *
 * Adds afforms as contact summary tabs.
 */
function civicrm_admin_ui_civicrm_tabset($tabsetName, &$tabs, $context) {
  if ($tabsetName !== 'civicrm/contact/view') {
    return;
  }

  $contactTypes = array_merge(['Contact'], \CRM_Contact_BAO_ContactType::basicTypes(FALSE));
  $customGroups = \Civi\Api4\CustomGroup::get(FALSE)
    ->addWhere('is_active', '=', TRUE)
    ->addWhere('style', '!=', 'Inline')
    ->addWhere('extends', 'IN', $contactTypes)
    // TODO: handle single value tabs
    ->addWhere('is_multiple', '=', TRUE)
    ->addSelect('name', 'style', 'extends', 'title', 'icon')
    ->execute();

  foreach ($customGroups as $group) {
    $tabId = 'custom_' . $group['id'];

    // check for old-style tab and remove it
    foreach ($tabs as $i => $tabMeta) {
      if ($tabMeta['id'] === $tabId) {
        unset($tabs[$i]);
        break;
      }
    }

    // only relevant for specific contact types, leave blank for all
    $contactType = ($group['extends'] !== 'Contact') ? $group['extends'] : NULL;
    $url = 'civicrm/contact/custom/' . $group['name'] . '/list#?contact_id=' . $context['contact_id'];
    $tabs[] = [
      'id' => $tabId,
      'title' => $group['title'],
      //'weight' => $afform['summary_weight'] ?? $weight++,
      'icon' => 'crm-i ' . ($group['icon'] ?: 'fa-list-alt'),
      'is_active' => TRUE,
      'contact_type' => $contactType,
      'url' => $url,
      //'template' => 'afform/contactSummary/AfformTab.tpl',
      //'module' => $afform['module_name'],
      //'directive' => $afform['directive_name'],
    ];
    // ensure the module required on that route is loaded
//    Civi::service('angularjs.loader')->addModules('afsearchCustom_' . $group['name']);
  }
}

I got got by the contact summary seemingly force the url to begin /civicrm/contact , so not happy with a generic url for a listing that works to any custom group (I had /civicrm/af/custom/GROUPNAME/list ).

Comment on lines +16 to +17
* - a submission form for adding a new Custom record to a parent entity record
* - a submission form for editing a particular Custom record
Copy link
Member

Choose a reason for hiding this comment

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

I think these 2 could be the same form actually.
entity_id could be a hidden field passed through the url.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yes I see what you mean, good point

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've kept separate forms - I think it makes sense that there's a clear route for the Create action vs Update - because entity_id is required for the former, but can be derived from the id (ie the custom record id) in the latter case.

Whilst at this point the entity_id is hidden on both forms, I would like to keep open the door for the entity_id being shown in future - probably readonly on the Update form; and potentially as an EntityRef select on the Create form.

I have combined the smarty templates for now though.

@colemanw
Copy link
Member

Got a bit bogged down in hook_civicrm_tabset and the afform implementation thereof though.
I wondered about just using an afform with a url to add the tabs directly, rather than via the afform placement tag?

I don't think you need to reinvent the wheel here. I can answer your questions about hook_civicrm_tabset. I'm sure it can already do what we want here.

@ufundo ufundo force-pushed the custom-group-create-update-afforms branch from ac868b3 to 6810ead Compare November 20, 2024 14:32
@ufundo ufundo force-pushed the custom-group-create-update-afforms branch from 6810ead to 80ed4e0 Compare November 20, 2024 19:19
@ufundo
Copy link
Contributor Author

ufundo commented Nov 20, 2024

I don't think you need to reinvent the wheel here. I can answer your questions about hook_civicrm_tabset. I'm sure it can already do what we want here.

I've got it working... though with a small hack, because the pre-existing custom tabs use the CustomGroup id, and I would really like the afforms to use CustomGroup name.

@ufundo ufundo marked this pull request as ready for review November 20, 2024 19:31
@ufundo
Copy link
Contributor Author

ufundo commented Nov 20, 2024

I think is ready for review now. Follow up coming with the tabs bit...

@colemanw
Copy link
Member

@ufundo I started reviewing this on the demo site & created a multi-record custom set & enabled the civicrm_admin_ui module but the tab on the contact summary screen remained the same. I must have misread your comment about a hack to change the existing custom tab, I thought you'd done it already.

But here's an interesting tidbit. When you use the old tab to create a new custom record, you see this:

image

So I guess that answers the question of "how did we get here?" - the old multi-record custom form triggers it. All the more reason to get rid of it.

And another interesting tidbit for you: if you didn't see it already, creating an Afform with the same name as a contact summary tab automatically replaces that tab, no need to do anything with hook_civicrm_tabSet. Although if you're bothered about the name of the tab being custom_4, then yes you'll need to use the hook to disable it manually.

@ufundo
Copy link
Contributor Author

ufundo commented Nov 21, 2024

@colemanw sorry there were a few stings in the tail of the follow up PR, which actually implements the Tabs.

But it is here now: #31503

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