Problem/Motivation

On Alert (Content Type) setting up Page visibility condition failed validation on the first line and doesn't continue to the next url for validation

Steps to reproduce

Create/Update an Alert (Content Type)
Test 1:
------
Set Page Visibility
/page1
/page1/*

Navigate to the page /page1 - Alert will not show.

Test 2:
------
Set Page Visibility
/page1/*
/page1

Navigate to the page /page1 - Alert will show on page1 page.

Proposed resolution

Validate all Page visibility url(s) to find the first valid (true) validation.
Don't break the loop when finding the first false
Break the loop on the first true validation (as you don't need to continue)

Remaining tasks

User interface changes

API changes

Data model changes

Comments

kobusvw created an issue. See original summary.

joshua1234511’s picture

joshua1234511
Primary language English
Location Goa
commented

Issue fixed in https://github.com/civictheme/uikit/pull/346

richardgaunt’s picture

richardgaunt commented

The fix updates the parsing of the incorrect "plain text" from the API but the root cause of this issue is in the `plain_text` field formatter which adds the twig filter `nlbr` to its processing.

https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...

The ideal fix will be to change the page visibility field to a multi-value plain text field so it provides an array of page visibility values and we can iterate over the values.

Due to this requiring a new field / update existing field and an upgrade path this will need to be considered.

For the meantime Josh's fix mitigates the issue

fionamorrison23’s picture

fionamorrison23 commented
Assigned: Unassigned » richardgaunt
fionamorrison23’s picture

fionamorrison23 commented
Issue tags: +civictheme-release-1.9
richardgaunt’s picture

richardgaunt commented

Sorry on further inspection I see that we are using a REST display of alert content types see: /admin/structure/views/view/civictheme_alerts

The `visibility` field needs to be updated to strip tags to remove the `
` tags.

Same underlying cause but we have ability to change field output.

richardgaunt’s picture

richardgaunt commented
Assigned: richardgaunt » joshua1234511
fionamorrison23’s picture

fionamorrison23 commented
Assigned: joshua1234511 » richardgaunt
Status: Active » Needs review
richardgaunt’s picture

richardgaunt commented

https://github.com/civictheme/uikit/pull/346
I have left a few questions, Josh

richardgaunt’s picture

richardgaunt commented
Status: Needs review » Needs work
fionamorrison23’s picture

fionamorrison23 commented
Status: Needs work » Needs review
sonam.chaturvedi’s picture

sonam.chaturvedi commented
Status: Needs review » Fixed

Verified and tested on 1.8-dev

Testing Results:
When we set Page visibility condition on Alert content type then validation does not fail for any urls listed - PASS

Screenshot:
Only local images are allowed.

Only local images are allowed.

sonam.chaturvedi’s picture

sonam.chaturvedi commented
Status: Fixed » Reviewed & tested by the community
fionamorrison23’s picture

fionamorrison23 commented
Issue tags: -civictheme-release-1.9 +civictheme-release-1.8.2
fionamorrison23’s picture

fionamorrison23 commented
Assigned: richardgaunt » Unassigned
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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

fionamorrison23’s picture

fionamorrison23 commented
Status: Closed (fixed) » Active

@richard noting this issue closed via automation. Kobus tells me this is not in the code base. I will need to check on other tickets that have automatically closed.

fionamorrison23’s picture

fionamorrison23 commented
Status: Active » Fixed

Status: Fixed » Closed (fixed)

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