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

Throw error on event creation if start date is later than end date #355

Merged
merged 21 commits into from
Oct 21, 2023

Conversation

maxwn04
Copy link
Contributor

@maxwn04 maxwn04 commented Jun 5, 2023

Added condition in the create function for the event service to check if the end date was later than the start date

Closes #179.

@github-actions
Copy link

github-actions bot commented Jun 5, 2023

Thanks for contributing! If you've made changes to the API's functionality, please make sure to bump the package version—see this guide to semantic versioning for details—and document those changes as appropriate.

tests/admin.test.ts Outdated Show resolved Hide resolved
services/EventService.ts Show resolved Hide resolved
tests/admin.test.ts Outdated Show resolved Hide resolved
tests/admin.test.ts Outdated Show resolved Hide resolved
tests/admin.test.ts Outdated Show resolved Hide resolved
compose-dev.yaml Outdated Show resolved Hide resolved
docker-machine Outdated Show resolved Hide resolved
yarn-error.log Outdated Show resolved Hide resolved
Copy link
Contributor

@dowhep dowhep left a comment

Choose a reason for hiding this comment

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

This PR is close to being done, just a few more changes as well as linting and removing unnecessary files.

tests/admin.test.ts Outdated Show resolved Hide resolved
@maxwn04
Copy link
Contributor Author

maxwn04 commented Jul 8, 2023

image

@dowhep dowhep self-requested a review July 14, 2023 04:37
Copy link
Contributor

@dowhep dowhep left a comment

Choose a reason for hiding this comment

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

A few nitpicks just to make sure the code base is extreeeemely consistent.

tests/admin.test.ts Outdated Show resolved Hide resolved
services/EventService.ts Outdated Show resolved Hide resolved
tests/admin.test.ts Outdated Show resolved Hide resolved
tests/admin.test.ts Outdated Show resolved Hide resolved
tests/admin.test.ts Outdated Show resolved Hide resolved
tests/admin.test.ts Outdated Show resolved Hide resolved
tests/admin.test.ts Outdated Show resolved Hide resolved
api/controllers/EventController.ts Outdated Show resolved Hide resolved
tests/admin.test.ts Outdated Show resolved Hide resolved
services/EventService.ts Outdated Show resolved Hide resolved
services/EventService.ts Outdated Show resolved Hide resolved
tests/admin.test.ts Outdated Show resolved Hide resolved
tests/admin.test.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@dowhep dowhep left a comment

Choose a reason for hiding this comment

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

Generally good, I think you will need to set up automating the test for the new test file (add it to test/index.ts). After you are done, verify that yarn test will run all the unit tests in the new test file.

services/EventService.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@dowhep dowhep left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@maxwn04 maxwn04 merged commit 21031e2 into master Oct 21, 2023
5 checks passed
@nik-dange nik-dange deleted the Max/check-event-date branch March 31, 2024 18:40
nick-ls pushed a commit to nick-ls/membership-portal that referenced this pull request Aug 6, 2024
…cmucsd#355)

* Throw error on event creation if start date is later than end date

* Test for create event with invalid date

* Remove unused imports, add trailing commas, add missing semi-colon

* Remove unecessary quotes

* Add EOL at end

* Add event testers for success and duplicate code case

* fix linter issues

* Permissions test for event creation

* Added event creation into repo check. Some Comments

* linter error fix

* more linter

* last check

* bugfix

* check event lookup

* fix services:

* lint

* more lint

* lint fix

* fix moment

* boolean fix
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.

Throw error on event creation if event start date is later than end date
3 participants