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

verifyContestModerator middleware #82

Merged
merged 20 commits into from
Dec 15, 2020

Conversation

ridhishjain-zepto
Copy link
Collaborator

@ridhishjain-zepto ridhishjain-zepto commented Dec 15, 2020

Issue at work: #60

resolves: #81

APIs covered:

  • createContest
    POST /contests/ Admin only: create a contest
  • updateContest
    POST /contests/:contest_id/update Moderator only: update contest
  • addModerator
    POST /contests/:contest_id/moderator Moderator only: Add moderator
  • removeModerator
    DELETE /contests/:contest_id/moderator Moderator only: remove moderator (creator can't be deleted)
  • addGroup
    POST /contests/:contest_id/:groups Moderator only: Add group
  • removeGroup
    DELETE /contests/:contest_id/groups Moderator only: remove group

Copy link
Collaborator

@cjchirag7 cjchirag7 left a comment

Choose a reason for hiding this comment

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

LGTM!

@ridhishjain-zepto
Copy link
Collaborator Author

I'll wait for @NBNARADHYA before merging 😆

server/models/contests/addGroup.js Outdated Show resolved Hide resolved
server/models/contests/addGroup.js Outdated Show resolved Hide resolved
server/models/contests/addModerator.js Outdated Show resolved Hide resolved
server/models/contests/addModerator.js Outdated Show resolved Hide resolved
server/models/contests/addModerator.js Outdated Show resolved Hide resolved
server/routes/contests/addGroup.js Outdated Show resolved Hide resolved
server/routes/contests/addModerator.js Outdated Show resolved Hide resolved
server/routes/contests/removeGroup.js Outdated Show resolved Hide resolved
server/routes/contests/removeModerator.js Outdated Show resolved Hide resolved
server/routes/contests/updateContest.js Outdated Show resolved Hide resolved
server/models/contests/addGroup.js Outdated Show resolved Hide resolved
server/models/contests/addGroup.js Outdated Show resolved Hide resolved
server/models/contests/createContest.js Outdated Show resolved Hide resolved
server/models/contests/addGroup.js Outdated Show resolved Hide resolved
server/models/contests/addGroup.js Outdated Show resolved Hide resolved
server/models/contests/createContest.js Outdated Show resolved Hide resolved
server/models/database/index.js Outdated Show resolved Hide resolved
server/models/contests/createContest.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@cjchirag7 cjchirag7 left a comment

Choose a reason for hiding this comment

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

Mainly, 2 changes are required:

  1. Release the pool connection in transaction, when any error occurs
  2. When the user who is sending the request doesn't have moderator access, appropriate error needs to be returned

server/models/contests/addGroup.js Show resolved Hide resolved
server/models/contests/addModerator.js Show resolved Hide resolved
server/models/contests/createContest.js Show resolved Hide resolved
server/models/contests/createContest.js Show resolved Hide resolved
server/models/contests/createContest.js Show resolved Hide resolved
server/models/contests/createContest.js Show resolved Hide resolved
server/models/contests/createContest.js Show resolved Hide resolved
server/models/contests/removeGroup.js Show resolved Hide resolved
server/models/contests/removeModerator.js Show resolved Hide resolved
Copy link
Collaborator

@cjchirag7 cjchirag7 left a comment

Choose a reason for hiding this comment

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

Looks clean

Copy link
Contributor

@NBNARADHYA NBNARADHYA left a comment

Choose a reason for hiding this comment

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

LGTM

@cjchirag7 cjchirag7 merged commit 31d58c2 into Cyber-Labs:master Dec 15, 2020
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.

verifyModerator middlware for contests
4 participants