-
-
Notifications
You must be signed in to change notification settings - Fork 825
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
Form Builder - Assign a default route to new "Submission Forms" and "Search Forms" #31843
base: master
Are you sure you want to change the base?
Conversation
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
@totten the more I think about it & discuss it, the less I like my original PR so I've closed it. But really it's not that much hassle and so for now I'm just going to make it required: 8996df2 That takes the pressure off this & we can slow down and think about whether we really want it. |
@@ -97,7 +97,6 @@ | |||
} | |||
if (editor.mode === 'clone') { | |||
delete editor.afform.name; | |||
delete editor.afform.server_route; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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:
- In the PHP side,
loadAdminData
prepares the definition of the new form. The PHP side assigns a newserver_route
(civicrm/form/XXXX-XXXX-XXXX
) for the clone. - 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.
- Before: PHP
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).
…ant) In this variant of the commit, we consolidate cloning logic on PHP side.
d249a99
to
c0854e8
Compare
Overview
Compare with #31834.
Before
After
When you make a "New Submission Form" or "New Search Form" (or "Clone" an existing one), it starts out with a real URL. Specifically:
civicrm/form/XXXX-XXXX-XXXX
civicrm/search/XXXX-XXXX-XXXX
The value is random but completely editable.
Comments
When you first make a new screen, you may be going back/forth in your own mind about what the screen should do. In effect, you are drafting the document. As a draft, you don't want random people to guess+access. But you do want it to be sufficiently live that you can use it (and maybe share with a colleague). The random ID is a very simple way to make it live without making it de facto public.
This patch does not affect "Field Block" forms. For that scenario, you still start out unpublished. And unpublished items are not web-addressable.
This patch doesn't require any changes to data-model or routing mechanics.