Problem/Motivation

Currently the cancel appointment form requires manual input of the appointment id and user's email address to cancel an appointment.
It would be better if the user can cancel an appointment directly from the link in the confirmation email, without the required manual input.

Proposed resolution

Create a token which generates a "cancel appointment" link for the confirmation email. When the link is called, the manual input step is skipped and the user only has to click the button to cancel the appointment.

CommentFileSizeAuthor
#4 interdiff-2-4.txt8.5 KBtvoesenek
#4 dvg_appointments-cancel_appointment-3085481-4.patch22.62 KBtvoesenek
#2 dvg_appointments-cancel_appointment-3085481-2.patch22.54 KBtvoesenek

Comments

TVoesenek created an issue. See original summary.

tvoesenek’s picture

tvoesenek commented
Status: Active » Needs review
StatusFileSize
new dvg_appointments-cancel_appointment-3085481-2.patch22.54 KB

I've added a new custom form to cancel an appointment, based on the solution in #3074229: Add appointment changing functionality for the "change appointment"-form.
For backwards compatibility, on the admin page you may now select if you want to use the "webform" or the "custom" solution for cancelling appointments.

When the "custom" solution is selected, a new token [submission:dvg_appointments_cancel_url] is available which can be used in the appointment confirmation email message to render a link to the "cancel"-form, with prefilled appointment data. It also has the option to specify the path of the form in all enabled languages.

The custom form falls back on the manual input if no appointment data is provided in the url parameters.

paulvandenburg’s picture

paulvandenburg commented
Issue summary: View changes
Status: Needs review » Needs work

There are a few issues that should be solved:

  1. You've defined 2 constants to configure what is essentially a boolean setting. Storing true/false should be sufficient. Also the defined constants misspelled the module's name, it is dvg_appointments
  2. You've recycled the change form access callback for the cancel urls, which in itself is fine, but it looks wrong. I think a comment should be added to explain why it is not wrong. Or rename the callback, like "dvg_appointments_form_token_access"
  3. At line 155 of the .module there is a typo "optiona" -> "optional"
  4. Why have you inverted the if logic of the token handling of the change url? Now the exceptional case is placed first and the main case second. More a code style thing, because it is still functional.
  5. There is an unsafe unserialize of possible user input in the cancel form (see line 59 of dvg_appointments.cancel.inc). The second param should be specified and should either disable unserializing objects, or mention the specific allowed objects.
  6. Typo at the end of the cancel.inc: submit handler is 2 words.

Overall looks like a great addition if the few mentioned issues are resolved.

tvoesenek’s picture

tvoesenek commented
Status: Needs work » Needs review
StatusFileSize
new dvg_appointments-cancel_appointment-3085481-4.patch22.62 KB
new interdiff-2-4.txt8.5 KB

Based on paulvandenburg's review I've made the following changes to the patch:
1. Replaced the constants with a boolean variable, set by a checkbox in the admin form.
2. Renamed the access callback, as suggested.
3. Fixed the typo.
4. Probably the result of some refactoring to prevent duplicate code, I agree it looks better when inverted.
5. Specified the allowed object, also on line 303 of dvg_appointments.cance.inc
6. Fixed the typo.

paulvandenburg’s picture

paulvandenburg commented
Status: Needs review » Reviewed & tested by the community

Looks good and works good!

Only 1 very small thing, which I've changed in the commit; In the dvg_appointments_form_cancel_appointment() function you used the short list() syntax. For consistency I've changed that to the longer "list()" version.

paulvandenburg’s picture

paulvandenburg commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.