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

fix: abp6 custom validation #686

Closed
wants to merge 4 commits into from
Closed

fix: abp6 custom validation #686

wants to merge 4 commits into from

Conversation

daniel-belcher
Copy link
Collaborator

Purpose

Fixing custom validation needs for ABP6.

Linked Issues to Close

ticket to close

Approach

Add custom validation tags for forms that will grab what is needed at form generation.

Assorted Notes/Considerations/Learning

Initial plan was to use string parsing similar to how we handle regex in the webform api, but this exposes us to injection for function handling. Tag system could become cumbersome if we have too many custom validations, but this is not foreseen to be the current plan for webforms.

Copy link

codeclimate bot commented Jul 26, 2024

Code Climate has analyzed commit 2c68f6d and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1

The test coverage on the diff in this pull request is 99.3% (50% is the threshold).

This pull request will bring the total coverage in the repository to 40.2% (0.3% change).

View more on Code Climate.

@Enterprise-CMCS Enterprise-CMCS deleted a comment from codeclimate bot Jul 26, 2024
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I truly love that you got this to work and we can define rules via type in a switch statement. I think its a great solution, and im stoked we have the ability to do custom validations now it really makes me feel more confident in our ability to say yes to those one off requests. thanks for tracking this down D 🥇

Copy link
Collaborator

@jdinh8124 jdinh8124 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome thank you for getting this for us

@benjaminpaige
Copy link
Collaborator

I think i added most of this code to the main branch cause i wanted to see it work. But i still cant get it to validate, im sure im doing something wrong. All that to say, if you wanted to use 'main' and figure out where the bug is and close this pr it might be easier than trying to figure out the deploy issue here

@daniel-belcher daniel-belcher deleted the cust-val-wfgen branch July 30, 2024 09:34
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.

3 participants