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

Refactor pipeline form parsing #109

Merged
merged 15 commits into from
Apr 17, 2024
Merged

Conversation

Ziyang-98
Copy link
Collaborator

@Ziyang-98 Ziyang-98 commented Apr 4, 2024

Changelogs:

Backend

  • Updated BE Pipeline FormField with the following fields
    • Title as the display title for the form field
    • Description as the display description for the form field
    • Required to determine whether field is mandatory (Note that checkboxes field don't have this)
    • MinLength for input field minimum length
    • Options for select and checkboxes fields
    • Default as the default option for select field
  • Implemented more validations for FormField
  • Added validation FormField in handleCreatePipeline which returns error if validation fails

Frontend

  • Refactored types for JsonFormComponent to mirror BE FormField model and updated all related objects to follow types
  • Updated rjsf parsing methods to convertform object into rjsf formSchema and uiSchema
  • Implemented more validations for form when creating a pipleine

Bruno

  • Added the fetch all pipeline API

Notes

  • Right now the BE returns a generic Bad Request error message for handleCreatePipeline. Would be better if the BE can return the validation message directly, but I'm not sure how to return custom error messages in Go. @joshtyf need your advice and opinion on this.
  • Currently the FE has more validations than BE. (for e.g., FE checks for whether form field names are unique while BE doesn't) I didn't implement those validations in the BE for now as I thought that the current ones are sufficient for an MVP, but in the future the validations in BE can be supplemented to match the FE.

Demo

Demo for the flow of creating a Pipeline with Form to displaying of the Form in Service Request page. Showcased some of the validation too.

2024-04-13.17-15-43.mp4

Closes #92

@Ziyang-98 Ziyang-98 changed the title [DRAFT] Refactor pipeline form parsing fe [DRAFT] Refactor pipeline form parsing Apr 4, 2024
Copy link
Owner

@joshtyf joshtyf left a comment

Choose a reason for hiding this comment

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

Changes are fine, but not really sure the intent behind it.

backend/src/database/models/pipeline.go Show resolved Hide resolved
backend/src/validation/validator.go Outdated Show resolved Hide resolved
backend/src/database/models/pipeline.go Outdated Show resolved Hide resolved
@Ziyang-98 Ziyang-98 changed the title [DRAFT] Refactor pipeline form parsing Refactor pipeline form parsing Apr 13, 2024
@Ziyang-98 Ziyang-98 requested a review from joshtyf April 13, 2024 09:14
@joshtyf
Copy link
Owner

joshtyf commented Apr 16, 2024

Right now the BE returns a generic Bad Request error message for handleCreatePipeline. Would be better if the BE can return the validation message directly, but I'm not sure how to return custom error messages in Go. @joshtyf need your advice and opinion on this.

Let's do this in a separate PR as this is quite big already. It's a good suggestion. Let me create the change

Copy link
Owner

@joshtyf joshtyf left a comment

Choose a reason for hiding this comment

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

LGTM

@Ziyang-98 Ziyang-98 merged commit a4ba27a into main Apr 17, 2024
2 checks passed
@Ziyang-98 Ziyang-98 deleted the refactor/pipeline-form-parsing-fe branch May 26, 2024 05:51
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.

Form submission
2 participants