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

feat: set and update user role #151

Merged

Conversation

120EE0692
Copy link
Member

Solved or Linked Issues

Reference any and all issues solved and related to this pull request here. Mention if there are none.

Description and Changes

Please describe your pull request with all changes made.

Your checklist for this pull request

🚨Please review the guidelines for contributing to this repository.

  • Make sure you are requesting to pull a work/topic/feature/bugfix branch (right side). Don't request your default branch!
  • Make sure you are making a pull request against the development (left side). Also you should start your branch off default branch and follow the branch naming guidelines.
  • Check the commit's or even all commits' message styles matches our requested structure.
  • Check your code additions will fail neither code linting checks nor unit test.
  • I have added necessary documentation and mentioned the related issues (if appropriate)
  • I have added the appropriate labels, requested a review from a repository manager and assigned myself to this pull request.

Post merge checklist

@120EE0692 120EE0692 self-assigned this Oct 15, 2022
@120EE0692 120EE0692 added enhancement New feature or request hacktoberfest The Hacktober Fest Label labels Oct 15, 2022
server/schema/user/user.datasources.js Outdated Show resolved Hide resolved
server/schema/user/user.datasources.js Outdated Show resolved Hide resolved
server/schema/user/user.datasources.js Outdated Show resolved Hide resolved
server/schema/user/user.mutation.js Outdated Show resolved Hide resolved
server/schema/user/user.query.js Outdated Show resolved Hide resolved
server/schema/user/user.datasources.js Outdated Show resolved Hide resolved
server/schema/user/user.datasources.js Outdated Show resolved Hide resolved
DesignrKnight
DesignrKnight previously approved these changes Oct 16, 2022
Copy link
Member

@DesignrKnight DesignrKnight left a comment

Choose a reason for hiding this comment

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

Would wait for @rutajdash to review
The basic overview looks good.

Two things

  • This feature branch is not rebased from the latest base branch
    image
  • ESLint failure is there in existing code
    image

Also, it is highly recommended to have tests for the project. You can start incrementally with tests for new code you write and then slowly cover rest of code. JS code(without TS), that too for server side is risky.
CC: @120EE0692

@rutajdash
Copy link
Member

rutajdash commented Oct 20, 2022

Also, it is highly recommended to have tests for the project. You can start incrementally with tests for new code you write and then slowly cover rest of code. JS code(without TS), that too for server side is risky. CC: @120EE0692

@DesignrKnight
Yes, there are a lot of plans for adding tests to the code, even to reach the point of test-driven development someday but at the moment we aren't able to spare any time from mainstream updates and features as there is a lot remaining to build.

@120EE0692 120EE0692 added the safe-for-checks This pull request is safe for actions label Oct 22, 2022
Copy link
Member

@rutajdash rutajdash left a comment

Choose a reason for hiding this comment

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

Please complete the prior changes.

server/schema/user/user.mutation.js Outdated Show resolved Hide resolved
server/schema/user/user.query.js Outdated Show resolved Hide resolved
@rutajdash rutajdash merged commit 76a81ea into Monday-Morning:main Oct 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request hacktoberfest The Hacktober Fest Label safe-for-checks This pull request is safe for actions
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants