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.

Sign up for GitHub

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

Form Builder - Assign a default route to new "Submission Forms" and "Search Forms" #31843

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

Form Builder - Assign a default route to new "Submission Forms" and "Search Forms" #31843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b1e38c3
Form Builder - Assign a default route to new "Submission Forms" and "…
totten Jan 21, 2025
c0854e8
Form Builder - When cloning a form, assign it to a new path (PHP vari…
totten Jan 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 35 additions & 0 deletions ext/afform/admin/Civi/Api4/Action/Afform/LoadAdminData.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,15 @@ class LoadAdminData extends \Civi\Api4\Generic\AbstractAction {
*/
protected $skipEntities = [];

/**
* Set TRUE if creating a clone.
*
* Some properties (such as `name`, `title`, `server_route`) may be filtered.
*
* @var bool
*/
protected $clone = FALSE;

public function _run(\Civi\Api4\Generic\Result $result) {
$info = ['entities' => [], 'fields' => [], 'blocks' => []];
$entities = [];
Expand All @@ -49,6 +58,7 @@ public function _run(\Civi\Api4\Generic\Result $result) {
$info['definition'] = $this->definition + [
'title' => '',
'permission' => ['access CiviCRM'],
'server_route' => $this->createDefaultRoute('form'),
'layout' => [
[
'#tag' => 'af-form',
Expand All @@ -71,6 +81,7 @@ public function _run(\Civi\Api4\Generic\Result $result) {
$info['definition'] = $this->definition + [
'title' => '',
'permission' => ['access CiviCRM'],
'server_route' => $this->createDefaultRoute('search'),
'layout' => [
[
'#tag' => 'div',
Expand Down Expand Up @@ -218,6 +229,17 @@ public function _run(\Civi\Api4\Generic\Result $result) {
}
$info['blocks'] = array_values($info['blocks']);

if ($this->clone) {
unset($info['definition']['name']);
$info['definition']['title'] .= ' ' . ts('(copy)');
if (!empty($info['definition']['server_route'])) {
$info['definition']['server_route'] = $this->createDefaultRoute($info['definition']['type']);
}
if (!empty($info['definition']['navigation']['label'])) {
$info['definition']['navigation']['label'] .= ' ' . ts('(copy)');
}
}

$result[] = $info;
}

Expand Down Expand Up @@ -264,6 +286,19 @@ private function loadAvailableBlocks($entities, &$info, $where = []) {
}
}

/**
* @param string $type
* Ex: 'form' or 'search'
* @return string
* Ex: 'civicrm/form/abcd-1234-abcd'
*/
private function createDefaultRoute(string $type): string {
$randChars = fn() => \CRM_Utils_String::createRandom(4, 'abcdefghijklmnopqrstuvwxyz1234567890');
$id = implode('-', [$randChars(), $randChars(), $randChars()]);
$buckets = ['form' => 'civicrm/form/', 'search' => 'civicrm/search/'];
return ($buckets[$type] ?? $buckets['form']) . $id;
}

/**
* @return array[]
*/
Expand Down
1 change: 1 addition & 0 deletions ext/afform/admin/ang/afAdmin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
// Load data for gui editor
data: function($route, crmApi4) {
return crmApi4('Afform', 'loadAdminData', {
clone: true,
definition: {name: $route.current.params.name}
}, 0);
}
Expand Down
6 changes: 0 additions & 6 deletions ext/afform/admin/ang/afGuiEditor/afGuiEditor.component.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,12 +95,6 @@
if (!editor.afform) {
alert('Error: unknown form');
}
if (editor.mode === 'clone') {
delete editor.afform.name;
delete editor.afform.server_route;
Copy link
Contributor

Choose a reason for hiding this comment

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

if I've understood this correctly, this will preserve the existing route when you clone a form? what happens if you (try to) save two forms with the same route?

Copy link
Member Author

Choose a reason for hiding this comment

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

In isolation, that would be true. But d249a99 has (had) two parts:

  1. In the PHP side, loadAdminData prepares the definition of the new form. The PHP side assigns a new server_route (civicrm/form/XXXX-XXXX-XXXX) for the clone.
  2. On the JS side, after it retrieves a definition, it needs to present it. This JS line blocked it from presenting the new route.
    • Before: PHP loadAdminData was feeding duplicate routes, so JS needed to discard those
    • After: PHP loadAdminData is feeding usable routes, so JS can keep them.

To your 2nd question, I'm not sure what happens if you make a duplicate in general. But I'm sure that the risk is much lower here than with the status-quo/chosen paths.

(My math is rusty, but it looks like 62-bits of entropy in the formula. For comparison, the mix of English words that a user would brainstorm for mnemonic URLs is... a ballpark of... 10-20 bits, generously speaking?)

Aside, after re-reading that commit, it looks like I had some rose-colored glasses and created a different bug (trampling the route on normal edits). Updating the clone operation isn't quite as elegant as updating "New Form" operation.

Posted updated flavors -- eg chocola PHP (c0854e8) or JS (totten@c6c7d85).

delete editor.afform.navigation;
editor.afform.title += ' ' + ts('(copy)');
}
editor.afform.icon = editor.afform.icon || 'fa-list-alt';
editor.afform.placement = editor.afform.placement || [];
$scope.canvasTab = 'layout';
Expand Down