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

CiviCRM WordPress shortcode remove the display of default text and instead just return blank if the shortcode cannot be rendered. #262

Merged
merged 1 commit into from
Dec 2, 2021

Conversation

agileware-justin
Copy link
Contributor

Overview

CiviCRM WordPress shortcode remove the display of default text and instead just return blank if the shortcode cannot be rendered.

Before

The message "Do not know how to handle this shortcode" or "The Shortcode could not be handled. It could be malformed or used incorrectly" can be shown publicly on a website (as well as to logged in users) if any of these conditions are true:

  1. If the user does not have the required permission to view the CiviCRM component, for example: no permission to view a specific afform page
  2. CiviCRM shortcode returns without setting a value
  3. CiviCRM shortcode is invalid, parameters are incorrect or missing

After

Shortcode returns blank. No error text is shown.

Technical Details

Comments

It would be preferred that error messages are not shown on the public website and if the CiviCRM shortcode has been mis-configured then this should be picked up by the CiviCRM Admin and/or CiviCRM Developer / Web Developer when testing the page.

It might be beneficial to add throw an exception and log an error when the CiviCRM shortcode returns an empty value. But that's beyond the scope of this PR.

Agileware Ref: CIVICRM-1896

…t text and instead just return blank if the shortcode cannot be rendered
@civicrm-builder
Copy link

Can one of the admins verify this patch?

@seamuslee001
Copy link
Contributor

Jenkins add to whitelist

@kcristiano
Copy link
Member

Unrelated Test failure.

@kcristiano
Copy link
Member

@christianwach Only improvement could be conditionally showing the warning to admins only. It would be better in than out in my opinion.

@christianwach
Copy link
Member

@kcristiano I agree, but this default is better.

@christianwach christianwach merged commit 64b5f07 into civicrm:master Dec 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants