-
-
Notifications
You must be signed in to change notification settings - Fork 827
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
Separate delete participant form from edit participant form #27660
Conversation
🤖 Thank you for contributing to CiviCRM! ❤️ We will need to test and review this PR. 👷 Introduction for new contributors...
Quick links for reviewers...
|
{$form.delete_participant.html} | ||
{/if} | ||
{else} {* If action is other than Delete *} | ||
{if 1} {* If action is other than Delete *} |
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.
the if can go later - but getting that right in tpls can be error prone so didn't want to do it here
5595b54
to
107aace
Compare
e2501a2
to
e03ee8d
Compare
@colemanw I'm wondering if you can help here - I have a hold-out problem - the delete button on the view form - I want that to load the delete code more directly.... UPDATE - got it working |
@eileenmcnaughton we have a lot of overloaded pages in CiviCRM that embed the form in the page for really no good reason. I've been tackling this in various places and I'm glad to see you making this change here. I'd love to get rid of them all. <item>
<path>civicrm/admin/reltype</path> <!-- view, add, update, delete -->
<page_callback>CRM_Admin_Page_RelationshipType</page_callback>
</item> And break it apart into 2 paths: <item>
<path>civicrm/admin/reltype</path> <!-- view -->
<page_callback>CRM_Admin_Page_RelationshipType</page_callback>
</item>
<item>
<path>civicrm/admin/reltype/edit</path> <!-- add, update, delete -->
<page_callback>CRM_Admin_Form_RelationshipType</page_callback>
</item> In the above (real) example, the form itself is still overloaded with multiple actions, but at least it's not embedded inside an overloaded page -- that's the most heinous part. |
@colemanw hmm - from the point of view of making the form less brittle moving this functionality and the overload functionality that loads the fee information by ajax are the priorities. This does the first of those & I would follow up with the second. The reality is that we probably could replace the extracted form with a SearchKit form pretty quickly if we choose - but it will take a while longer to replace the main form which is 'freed up' by getting this functionality out of it. So I think if the objective is to support php8.x, get rid of smarty notices & make the sending of workflow messages independent of the form layer (my goal) - this moves us in that direction. At the moment we have the need to retrieve consistent values like event id & participant id & that retrieval works differently in the different forms jammed into I think it does take us forwards on the part that you think is most heinous but is probably a side issue from my point of view. |
Well it's progress either way. And now you know what I think about overloading pages with embedded forms 🤮 |
Overview
Separate delete participant form from edit participant form
Before
The delete participant action and the load price field action are overloaded onto the edit participant form - in part via the confusing
EventTab
class. This makes the code really hard to follow.After
I separated the delete participant form to have it's own path..
Technical Details
Note that
Comments