Skip to content

Add new input rule for ShowOn field#41460

Merged
HLeithner merged 3 commits intojoomla:5.0-devfrom
magnussinger:i41217
Sep 2, 2023
Merged

Add new input rule for ShowOn field#41460
HLeithner merged 3 commits intojoomla:5.0-devfrom
magnussinger:i41217

Conversation

@magnussinger
Copy link
Copy Markdown
Contributor

Pull Request for Issue #41217.

Summary of Changes

Added a new input rule in oder to check if the show on input text is valid. If not, the user is unable to safe the field

Testing Instructions

Create a new custom field and edit the Showon Attribute. Enter something invalid and you should be unable to save. If you enter something valid, the field can be saved.

Actual result BEFORE applying this Pull Request

The user was able to save the field independent on what's in the showon attribute.

Expected result AFTER applying this Pull Request

The user can only save the field when the showon rule is valid

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@rdeutz
Copy link
Copy Markdown
Contributor

rdeutz commented Aug 26, 2023

This should be backed up with tests, one reason is that this is some kind of documentation. And further more it needs documentation.

Co-authored-by: Richard Fath <richard67@users.noreply.github.com>
@chmst
Copy link
Copy Markdown
Contributor

chmst commented Aug 26, 2023

I have tested this item ✅ successfully on f402a7b


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/41460.

@crommie
Copy link
Copy Markdown

crommie commented Aug 26, 2023

I have tested this item ✅ successfully on f402a7b

Tested succesfully, I got "Invalid field: Showon Attribute" when I entered something random


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/41460.

@chmst
Copy link
Copy Markdown
Contributor

chmst commented Aug 26, 2023

Tested successfully. Not sure if this needs documentation as it was a bug.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/41460.

@richard67
Copy link
Copy Markdown
Member

Not sure if this needs documentation as it was a bug.

It adds a new validation rule, so it the available validation rules are listed somewhere in our documentation or are even described, this needs to be extended by the new rule.

@richard67
Copy link
Copy Markdown
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/41460.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 26, 2023
@rdeutz
Copy link
Copy Markdown
Contributor

rdeutz commented Aug 26, 2023

Sorry, I should have explained it better, with tests I meant system tests and not manual testing.

@richard67 richard67 added the bug label Aug 26, 2023
@HLeithner HLeithner merged commit e4ad4a8 into joomla:5.0-dev Sep 2, 2023
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 2, 2023
@HLeithner
Copy link
Copy Markdown
Member

thanks, adding a test for this would be great.

@obuisard
Copy link
Copy Markdown
Contributor

Should work:
box:value1
box!:value1
box!:
box:value1[OR]square:value1
box:value1[AND]square:value1
box:value1[OR]square!:value1
box:value1[AND]square!:value1
box:value1[AND]square:value1,value2
box:value1[AND]square!:value1,value2

Should fail:
tada,
box::,
box:!,
box:3:21,
[AND]box:value3[OR]tada:2:3

@richard67
Copy link
Copy Markdown
Member

@obuisard box!: (without a value specified) is currently failing, see issue #41788 .

@richard67
Copy link
Copy Markdown
Member

richard67 commented Sep 25, 2023

And box:3:21 and [AND]box:value3[OR]tada:2:3 pass the regex but they should fail. But this might be due to me having tested this with the complete regex and rule string using https://www.functions-online.com/preg_match.html , so I haven't cut the rule into parts separated by the AND and OR conditions like the code does. I've meanwhile done a real test, and they still pass, but they should fail.

@richard67
Copy link
Copy Markdown
Member

Am working on a fix, see #41918 . But I have to test it myself first. So it's still draft.

@richard67
Copy link
Copy Markdown
Member

@magnussinger What I don't understand it the $andRules = explode('&', $value); and the $orRules = explode('|', $andRule);. When I debug the value of the $value variable, it doesn't contain &or |, it contains [AND] and [OR] instead. Can it be that there is some "str_replace" missing for that? I think this thing is the reason why the [AND]box:value3[OR]tada:2:3 is detected to be valid, but it shouldn't?

@richard67
Copy link
Copy Markdown
Member

When fixing my previous comment by using $andRules = explode('[AND]', $value); and $orRules = explode('[OR]', $andRule);, the issue with [AND]box:value3[OR]tada:2:3 is solved. But the box:3:21 is not solved yet.

@richard67
Copy link
Copy Markdown
Member

I've fixed all these issues mentioned above. See #41918 .

@richard67
Copy link
Copy Markdown
Member

Sorry, I should have explained it better, with tests I meant system tests and not manual testing.

@rdeutz This PR here and my fix are things to be tested by unit tests, not by system tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants