[WiP] Textfield to support RegExp validation #459

drehelis · 2021-06-14T12:55:56Z

I was in a need for special form of text field validation done on the frontend. This PR tries to solve this by adding regular-expression validation capabilities.

Couple of caveats -

Escaping characters such as \ when editing JSON config manually is required.
~~Can't get the Admin UI properly generate regex object. Hence, WiP :-)~~
Add Tests

Parameter config should look like:

{
  "name": "RegExp textfield",
  "max_length": "12",
  "regex": {
    "pattern": "^([a-z0-9_\\.-]+)@([\\da-z\\.-]+)\\.([a-z\\.]{2,6})$",
    "description": "Email address only"
  }
}

Small demo:

Screen.Recording.2021-06-14.at.15.48.54.mov

bugy

Thanks, looks good to me, just a couple of improvement suggestions

bugy · 2021-06-16T07:22:00Z

src/model/parameter_config.py

@@ -279,6 +282,8 @@ def validate_value(self, value, *, ignore_required=False):
        if is_empty(value):
            if self.required and not ignore_required:
                return 'is not specified'
+            if self.regex:


I think this is redundant

bugy · 2021-06-16T07:23:52Z

src/model/parameter_config.py

+            if self.regex is not None:
+                regex_pattern = self.regex.get('pattern', None)
+                if (not is_empty(regex_pattern)):
+                    regex_matched = bool(re.match(regex_pattern, value))


I think bool is not needed here

bugy · 2021-06-16T07:24:37Z

web-src/src/admin/components/scripts-config/ParameterConfigForm.vue

@@ -73,6 +73,12 @@
                 title="Excluded files"
                 @error="handleError('Excluded files', $event)"/>
    </div>
+    <div v-if="selectedType === 'text' || selectedType === undefined" class="row">


May be you could combine it with the max length row?

You mean, move both regex_pattern & regex_description textfields under the same div with max_length?

bugy · 2021-06-16T07:26:02Z

web-src/src/common/components/textfield.vue

+        let regexPattern = new RegExp(regex.pattern);
+        let matchFound = regexPattern.test(value);
+        if (!matchFound) {
+          return regex.description


Since description can be empty, please provide a fallback value

When/if description is left empty, regex.description is the regex pattern itself. The idea is to provide the end-user with a meaningful error message when it comes to regex validation but if, for some reason, you don't, shouldn't print the regex pattern is enough of a fallback value?

script-server/src/model/parameter_config.py

Line 302 in 8ade772

return 'does not match regex pattern: ' + self.regex.get('description', regex_pattern)

The python code you are referencing is a backend validation, a user should never see it. It's just an additional safety net.

bugy · 2021-06-16T07:26:52Z

web-src/src/common/components/textfield.vue

  if (type === 'text') {
+    if (regex) {


Could you add a test to https://github.com/bugy/script-server/blob/master/web-src/tests/unit/textfield_test.js ?

How do I get started with tests? How do I run them locally on my dev env?
It would be nice to document this on the Wiki somewhere :-)

bugy · 2021-06-16T07:27:06Z

src/model/parameter_config.py

@@ -289,6 +294,13 @@ def validate_value(self, value, *, ignore_required=False):
            return None

        if self.type == 'text':
+            if self.regex is not None:


Could you add a couple of tests?

bugy · 2021-06-16T07:28:41Z

src/model/parameter_config.py

@@ -74,6 +76,7 @@ def _reload(self):
        self.min = config.get('min')
        self.max = config.get('max')
        self.max_length = config.get('max_length')
+        self.regex = config.get('regex')


Please also update the method "get_sorted_config" below

bugy · 2021-06-19T06:13:59Z

web-src/src/admin/components/scripts-config/ParameterConfigForm.vue

@@ -235,6 +249,8 @@ export default {
          this.min = config['min'];
          this.max = config['max'];
          this.max_length = config['max_length'];
+          this.regex_pattern = get(config, 'regex.pattern', '');


Please use camelCase for naming in javascript

ashishp0911 · 2022-11-27T12:23:26Z

Hello ,
This doesn't seem to work. I tried the exact pattern mentioned in this thread , but there doesn't seem to be any validation on the Input Field or am I missing something

The parameter section below : -

"parameters": [
{
"name": "Input",
"required": true,
"param": "-i",
"env_var": "INP",
"type": "text",
"regex": {
"pattern": "^([a-z0-9_\.-]+)@([\da-z\.-]+)\.([a-z\.]{2,6})$",
"description": "Input Validation"
}
}

Regards,
Ashish .A. Poojary

bugy · 2022-11-27T13:18:49Z

Hi @ashishp0911, how did you test it? This PR was not merged, i.e. not available in script server builds.

ashishp0911 · 2022-11-27T13:28:41Z

@bugy My Bad , I spoke too early. I mistook the closed case #466 and thought this feature has been implemented in the latest build.
Any updates on the rollout of this feature ?

# Conflicts: # src/model/parameter_config.py # src/tests/test_utils.py # web-src/src/admin/components/scripts-config/ParameterConfigForm.vue

…ther minor fixes

bugy · 2022-11-27T17:41:39Z

I have to apologize to @drehelis for missing that one. To be honest I was waiting for "review request", but I had to check this ticket on my own. Totally my fault for not tracking the project properly.

I made some minor adjustments, and will wait for build before merging. Probably tomorrow.

textfield regex validation

8ade772

bugy requested changes Jun 16, 2021

View reviewed changes

bugy requested changes Jun 19, 2021

View reviewed changes

drehelis added 2 commits June 19, 2021 12:20

Merge branch 'master' into regex_text

2737bbd

code review adaptations

09ff350

bugy mentioned this pull request Jul 5, 2021

regex parameter checking #466

Closed

bugy#459 added a couple of tests for regex validation + fixed small bugs

e3d1c89

bugy added 2 commits November 27, 2022 15:49

Merge branch 'master' into regex_text

07f2743

# Conflicts: # src/model/parameter_config.py # src/tests/test_utils.py # web-src/src/admin/components/scripts-config/ParameterConfigForm.vue

bugy#459 fixed regex validation to be performed on a whole string + o…

8b7993b

…ther minor fixes

bugy merged commit 081b20f into bugy:master Nov 29, 2022

bugy added feature resolved labels Nov 29, 2022

bugy added this to the 1.18.0 milestone Nov 29, 2022

drehelis deleted the regex_text branch November 29, 2022 19:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WiP] Textfield to support RegExp validation #459

[WiP] Textfield to support RegExp validation #459

drehelis commented Jun 14, 2021 •

edited

Loading

bugy left a comment

bugy Jun 16, 2021

bugy Jun 16, 2021

bugy Jun 16, 2021

drehelis Jun 16, 2021

bugy Jun 17, 2021

bugy Jun 16, 2021

drehelis Jun 16, 2021

bugy Jun 17, 2021

bugy Jun 16, 2021

drehelis Jun 18, 2021

bugy Jun 16, 2021

bugy Jun 16, 2021

bugy Jun 19, 2021

ashishp0911 commented Nov 27, 2022

bugy commented Nov 27, 2022

ashishp0911 commented Nov 27, 2022

bugy commented Nov 27, 2022

[WiP] Textfield to support RegExp validation #459

[WiP] Textfield to support RegExp validation #459

Conversation

drehelis commented Jun 14, 2021 • edited Loading

bugy left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ashishp0911 commented Nov 27, 2022

bugy commented Nov 27, 2022

ashishp0911 commented Nov 27, 2022

bugy commented Nov 27, 2022

drehelis commented Jun 14, 2021 •

edited

Loading