Skip to content

Conversation

@jscyo
Copy link
Contributor

@jscyo jscyo commented Apr 11, 2022

Summary of change

Refactors AddUserRole API in the user-roles base branch + fixs. Adds tests for AddUserRole API related functions.

Related issues

Test Plan

API tests:

  • badInputTests:
    • dont pass either roles or userId
    • don't pass userId
    • pass userId as a number
    • don't pass role
    • role as a number
    • role as an empty string
  • Add a role to the user, check that the user has the role
  • Add an unknown role to the user, check for UNKNOWN_ROLE_EXCEPTION, check that user does not have a role

Core functions test

  • test adding a role to user responses:
    • assigning an unknown role
    • assigning a role and checking if the user has the role
    • assigning the same role twice, should not throw an exception and there should not be a duplicate role
    • assign another role and check the user has two roles now

Storage tests

  • assign an unknown role to the user
  • assign multiple different roles to the user, check if the user has the roles
  • assign the same role twice and check for DuplicateRoleException and the user should not have duplicate roles

Documentation changes

(If relevant, please create a PR in our docs repo, or create a checklist here highlighting the necessary changes)

Checklist for important updates

  • [ ] Changelog has been updated
    • [ ] If there are any DB schema changes, mention those changes clearly
  • [ ] coreDriverInterfaceSupported.json file has been updated (if needed)
  • [ ] pluginInterfaceSupported.json file has been updated (if needed)
  • [ ] Changes to the version if needed
    • In build.gradle
  • Had installed and ran the pre-commit hook
  • [ ] If there are new dependencies that have been added in build.gradle, please make sure to add them in implementationDependencies.json.
  • Issue this PR against the latest non released version branch.
    • To know which one it is, run find the latest released tag (git tag) in the format vX.Y.Z, and then find the latest branch (git branch --all) whose X.Y is greater than the latest released tag.
    • If no such branch exists, then create one from the latest released branch.

Remaining TODOs for this PR

  • none

@jscyo jscyo self-assigned this Apr 11, 2022
@jscyo jscyo changed the title Add role to user api feat (userroles): AddUserRole api Apr 14, 2022
@jscyo jscyo changed the title feat (userroles): AddUserRole api feat: AddUserRole api Apr 14, 2022
@rishabhpoddar
Copy link
Contributor

rishabhpoddar commented Apr 15, 2022

Moving this back to in progress since the "run test" check is not passing..

@rishabhpoddar
Copy link
Contributor

Write down all test cases you have covered in the PR message above. Moving this back to in progress

@jscyo jscyo merged commit 01e57ea into user-roles Apr 18, 2022
@jscyo jscyo deleted the add-role-to-user-api branch April 18, 2022 07:16
rishabhpoddar added a commit that referenced this pull request May 6, 2022
* adds addUserRoleAPI

* renames functions for retriving tables

* fixs

* fixs, adds create index queries and create table queries to start file

* fixs

* fixs index name for role_permissions table

* feedback fixs

* test(userroles): Add create role api (#418)

* adds createRole API

* fixs

* adds more bad input tests

* adds more tests

* implements some feedback

* adds lock to doesRoleExist_Transaction

* adds some storage tests

* removes print statement

* adds some storage tests

* adds more tests

* fixs

* test fixs

* adds missing commitTransaction

* updates test

* fixs incorrect foreign key constraint names

* fixs

* userRole updates due to plugin-interface changes, Updates queries and tests

* adds getRoles test

* updates createNewRole query name

* test fixs

* renames addPermissionToRole function (#425)

* feat: AddUserRole api (#423)

* fixs

* fixs

* test, fixs

* adds more storage tests for addRoleToUser

* adds tests for addRoleToUser core function

* fixs sanitizing role in api

* adds badInputTests

* adds additional tests

* adds addtional storagetest to check UNKNOWN_ROLE_EXCEPTION

* test fixs

* fix: updates plugin-interface functions, test fixs (#428)

* fix: updates plugin-interface functions, test fixs

* adds remaining changes

* fixs

* updates function name

* test fixs

* changes didUserHaveRole to didUserAlreadyHaveRole

* feat: adds RemoveUserRoleAPI (#426)

* initial commit

* query fixs, adds storage tests

* adds userRoles tests

* adds removeUserRoleAPI

* adds removeUserRoleAPI

* updates RemoveUserRole api and function to now return a boolean on whether the user had the role

* adds serialVersion field for API

* adds test to check removing a role from a user, where the user did not have the role

* adds more core tests

* fixs

* adds FOR UPDATE to doesRoleExist_Transaction

* udpates to test

* fixs

* feedback fisx

* feat: get userroles api (#430)

* initial commit

* query fixs, adds storage tests

* adds userRoles tests

* adds removeUserRoleAPI

* adds removeUserRoleAPI

* updates RemoveUserRole api and function to now return a boolean on whether the user had the role

* adds serialVersion field for API

* adds test to check removing a role from a user, where the user did not have the role

* adds more core tests

* fixs

* adds FOR UPDATE to doesRoleExist_Transaction

* udpates to test

* fixs

* fixs

* test fixs

* fixs

* adds more core tests

* adds more core tests

* feedback fixs

* feat: adds GetUsersForRoleAPI + tests (#431)

* initial commit

* query fixs, adds storage tests

* adds userRoles tests

* adds removeUserRoleAPI

* adds removeUserRoleAPI

* updates RemoveUserRole api and function to now return a boolean on whether the user had the role

* adds serialVersion field for API

* adds test to check removing a role from a user, where the user did not have the role

* adds more core tests

* fixs

* adds FOR UPDATE to doesRoleExist_Transaction

* udpates to test

* fixs

* fixs

* test fixs

* adds test and query fix

* adds remaining storage test

* adds core function test

* temp commit

* fixs

* test fixs

* test fixs

* typo fix

* feat: adds GetPermissionsForRoleAPI + tests

* Revert "feat: adds GetPermissionsForRoleAPI + tests"

This reverts commit cb89fff.

* feedback fixs

* feat: adds GetPermissionForRoleAPI + tests (#432)

* initial commit

* query fixs, adds storage tests

* adds userRoles tests

* adds removeUserRoleAPI

* adds removeUserRoleAPI

* updates RemoveUserRole api and function to now return a boolean on whether the user had the role

* adds serialVersion field for API

* adds test to check removing a role from a user, where the user did not have the role

* adds more core tests

* fixs

* adds FOR UPDATE to doesRoleExist_Transaction

* udpates to test

* fixs

* fixs

* test fixs

* adds test and query fix

* adds remaining storage test

* adds core function test

* temp commit

* fixs

* test fixs

* test fixs

* typo fix

* feat: adds GetPermissionsForRoleAPI + tests

* fixs

* removes unnecessary print statments

* adds tests and some fixs

* feat: adds RemovePermissionsFromRoleAPI + tests (#433)

* adds core function

* adds RemovePermissionsForRoleAPI

* adds basic storage tests

* adds core function tests

* fixs

* removes unused imports

* fixs

* adds an additional api test

* adds missing transaction commit

* implements feedback

* feat: adds GetRolesForPermissionAPI + tests (#434)

* adds core function

* adds RemovePermissionsForRoleAPI

* adds basic storage tests

* adds core function tests

* fixs

* removes unused imports

* fixs

* adds an additional api test

* adds getRolesForPermissionAPI

* adds storage test

* adds core function tests

* adds API tests

* adds missing transaction commit

* implements feedback

* feat: adds RemoveRoleAPI + tests (#436)

* adds core function

* adds RemovePermissionsForRoleAPI

* adds basic storage tests

* adds core function tests

* fixs

* removes unused imports

* fixs

* adds an additional api test

* adds getRolesForPermissionAPI

* adds storage test

* adds core function tests

* adds API tests

* adds missing transaction commit

* implements feedback

* feat: Adds RemoveRoleAPI

* fixs + adds storage test

* adds api test

* adds core tests

* adds remaining API tests

* removes unused import

* implements feedback

* feat: adds GetRolesAPI + tests (#437)

* adds core function

* adds RemovePermissionsForRoleAPI

* adds basic storage tests

* adds core function tests

* fixs

* removes unused imports

* fixs

* adds an additional api test

* adds getRolesForPermissionAPI

* adds storage test

* adds core function tests

* adds API tests

* adds missing transaction commit

* implements feedback

* feat: Adds RemoveRoleAPI

* fixs + adds storage test

* adds api test

* adds core tests

* adds remaining API tests

* removes unused import

* fixs

* implements feedback

* Adds test

* adds additional tests

* feat: adds deleteAllRolesForUser + tests (#438)

* adds core function

* adds RemovePermissionsForRoleAPI

* adds basic storage tests

* adds core function tests

* fixs

* removes unused imports

* fixs

* adds an additional api test

* adds getRolesForPermissionAPI

* adds storage test

* adds core function tests

* adds API tests

* adds missing transaction commit

* implements feedback

* feat: Adds RemoveRoleAPI

* fixs + adds storage test

* adds api test

* adds core tests

* adds remaining API tests

* removes unused import

* fixs

* implements feedback

* Adds test

* adds additional tests

* adds core function for removing all roles for a user

* adds storage test

* adds core function tests

* adds deleteAllRolesForUser to deleteUser function

* adds additional test

* chore: remaining checklist todos (#439)

* updates build.gradle and CHANGELOG

* updates

* feedback fixs

* test: updates test (#440)

* updates CHANGELOG.md

Co-authored-by: Rishabh Poddar <rishabh.poddar@gmail.com>
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.

3 participants