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

present friendlier error message if petition cannot be displayed. #25915

Closed
wants to merge 1 commit into from

Conversation

jmcclelland
Copy link
Contributor

Overview

Provide a friendlier error message if a petition cannot be displayed for some reason.

Before

If a user clicks on a link to a petition that either doesn't exist, is disabled, or the link doesn't have a sid parameter - the user is redirected to the home page with an error message.

However, the home page may not be accessible to a non-logged in user, so instead of seeing the error, they simply get a forbidden message. Also, depending on how the theme is configured, they may get a page with a different theme than the petition page should display.

After

Instead of redirecting the user, display the petition page but without any of the petition elements. If the user has access to sign a petition, then they will stay on the page and get the friendly error message displayed.

Technical Details

The old code uses the CRM_Core_Error::statusBounce() call. That is replaced with a $session->setStatus() call. I wasn't sure if there is an existing way to get quick form to bail on displaying a form so I added a class variable. Maybe there is a better way?!?

@civibot
Copy link

civibot bot commented Mar 24, 2023

(Standard links)

@civibot civibot bot added the master label Mar 24, 2023
@demeritcowboy
Copy link
Contributor

I wasn't sure if there is an existing way to get quick form to bail on displaying a form so I added a class variable.

Historically it was CRM_Core_Error::fatal(), which was always available to all users/anonymous. But it's not very pretty/friendly either. But the concept of such a page seems like the right concept for this situation. You can get a similar thing with throw new CRM_Core_Exception(), but again not pretty/friendly, especially for the message about "petition no longer active". The others are unusual cases so the page wouldn't be seen that often, so for those throwing an exception seems fine. And maybe good enough for "no longer active" one for now, since then at least they see something?

@jmcclelland
Copy link
Contributor Author

Thanks for the feedback @demeritcowboy - the real goal of this merge is to avoid the confusion on the "no longer active" petitions.

I'm strongly encouraging our groups to disable petitions after the campaign is done (since these forms become spam magnets). However, I'm getting resistance to disabling them because when they are disabled, the groups gets a steady stream of concerned email messages from people who clicked on the link and get a forbidden message. The group gets tired of these emails so they re-enable the petition.

@demeritcowboy
Copy link
Contributor

Maybe could do it like how event info pages work when the event has passed? There's already something similar in the petition template here:

{if $duplicate == "confirmed"}
<p>
{ts}You have already signed this petition.{/ts}

So in the php don't bounce but assign something, then in the tpl display a message.

$this->assign('isActive', $this->petition['is_active']);
{if !$isActive}
  {ts}This petition is no longer active.{/ts}
{else}
  ... rest of template...
{/if}

@jmcclelland
Copy link
Contributor Author

Thanks for the suggestion! I just submitted a new PR.

@jmcclelland jmcclelland closed this Jun 5, 2023
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