Skip to content

Evn validation 1403#1440

Merged
trillium merged 1 commit intohackforla:developmentfrom
evanyang1:evn-validation-1403
Aug 1, 2023
Merged

Evn validation 1403#1440
trillium merged 1 commit intohackforla:developmentfrom
evanyang1:evn-validation-1403

Conversation

@evanyang1
Copy link
Member

@evanyang1 evanyang1 commented Jul 17, 2023

Fixes #1403

What changes did you make and why did you make them ?

  • When editing the event, the names will be validated. If the meeting contains 'meeting', 'mtg', or the project name itself, the validation code will reject those changes.
  • I wrote a new util function that serves the task: It returns true if a string contains any of the 'words' in an array.

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Details

@github-actions
Copy link

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b evanyang1-evn-validation-1403 development
git pull https://github.com/evanyang1/VRMS.git evn-validation-1403

@evanyang1 evanyang1 marked this pull request as ready for review July 17, 2023 06:18
@jbubar jbubar requested a review from trillium July 17, 2023 18:10
@trillium
Copy link
Member

Hey Evan,

Can you add a bit of context on the changes you've made to the codebase and why they've been made?

I know that we have the issue markes, and that you've referenced them within this PR, but it's good practice to put a summary together so that whoever is reviewing it has an easier time following along with the changes you've submitted.

@evanyang1
Copy link
Member Author

@spiteless I used a slightly different approach to address your comments. I tested it and it works still.

Copy link
Member

@jbubar jbubar 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 great! I pulled it and it looks good

pic of it live Screen Shot 2023-07-27 at 4 11 25 PM

@trillium
Copy link
Member

trillium commented Jul 31, 2023

Hey @evanyang1 , looks like merging is blocked becasue I requested changes and haven't come back yet to remove that change request. My bad.

Combing back through the PR I see that you addressed getting the form submission issue that was cropping up.

Looks like edits to yarn.lock are still in this pull request, can you please remove those, the changes you've made shouldn't effect that file

@evanyang1
Copy link
Member Author

Ok that should do the trick.

@evanyang1 evanyang1 force-pushed the evn-validation-1403 branch from 236d024 to 7074341 Compare July 31, 2023 23:18
@evanyang1 evanyang1 force-pushed the evn-validation-1403 branch from 7074341 to 28fdb34 Compare July 31, 2023 23:24
@trillium
Copy link
Member

Great job, thanks for rebasing and dropping yarn lock. We've got a few PRs that all tackle this similar issue so stitching them together will be it's own beast, but this code works. Thanks @evanyang1 !

@trillium
Copy link
Member

Looking for a few more eyes on this if someone else has time. I'm happy with it's current state

@trillium trillium requested a review from MattPereira July 31, 2023 23:29
Copy link
Contributor

@MattPereira MattPereira left a comment

Choose a reason for hiding this comment

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

@evanyang1 well done sanitizing the meeting name input according to the requirements of the issue!

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.

Event name validation: Add regex validation so that people cannot submit the wrong event name

4 participants