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.

Sign up for GitHub

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

Jump to bottom

Feat: CASL permission voyage sprints #193

Merged
merged 27 commits into from
Sep 16, 2024
Merged

Feat: CASL permission voyage sprints #193

merged 27 commits into from
Sep 16, 2024

Conversation

Ajen07
Copy link
Contributor

@Ajen07 Ajen07 commented Aug 31, 2024

Description

  • Add permissions for Voyage Sprint endpoint

Issue link

Fixes # (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature updates / changes
  • Tests
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • test with yarn test:docker
  • tested with swagger

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have updated the change log

Ajen07 added 13 commits August 26, 2024 14:23
@Ajen07
added ability for sprint meetings
113a927
@Ajen07
added permissions for GET /voyages/sprint and /voyages/sprint/teams/:…
999244b 
…teamId
@Ajen07
added swagger responses
fffc725
@Ajen07
added permissions for GET /voyages/sprints/meetins/:meetingId
115ecf5
@Ajen07
added permissions for PATCH /voyages/sprints/meetins/:meetingId
ae690f1
@Ajen07
added permissions for PATCH /voyages/sprints/:sprintNumber/teams/:tea…
8e4986c 
…mId/meetings
@Ajen07
added permissions for POST /voyages/sprints/meetings/:meetingId/agendas
2c65d8e
@Ajen07
Merge branch 'dev' of https://github.com/chingu-x/chingu-dashboard-be …
023d446 
…into feature/casl-permission-voyage-sprints
@Ajen07
added permissions for PATCH and DELETE /voyages/sprints/agendas/:agen…
33a5629 
…daId
@Ajen07
added permissions for PATCH POST GET /voyages/sprints/meetings/:meeti…
db78309 
…ngId/forms/:formId
@Ajen07
added swagger responses
2214db7
@Ajen07
refractored test
bf079cd
@Ajen07
updated changelog.md
1784e18
@Ajen07 Ajen07 added the feature label Aug 31, 2024
@Ajen07 Ajen07 self-assigned this Aug 31, 2024
@Ajen07 Ajen07 changed the title Feature: CASL permission voyage sprints Feat: CASL permission voyage sprints Aug 31, 2024
cherylli
cherylli reviewed Aug 31, 2024
View reviewed changes
Copy link
Contributor

@cherylli cherylli left a comment

Choose a reason for hiding this comment

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

looks good so far, what do you think about adding a new Action for the forms?

src/sprints/sprints.controller.ts Outdated Show resolved Hide resolved
src/sprints/sprints.controller.ts Outdated Show resolved Hide resolved
src/sprints/sprints.controller.ts Outdated Show resolved Hide resolved
src/sprints/sprints.controller.ts Outdated Show resolved Hide resolved
src/sprints/sprints.service.ts Outdated Show resolved Hide resolved
src/ability/ability.factory/ability.factory.ts Outdated Show resolved Hide resolved
timDeHof
timDeHof approved these changes Sep 1, 2024
View reviewed changes
Copy link
Contributor

@timDeHof timDeHof left a comment

Choose a reason for hiding this comment

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

All tests passed! everything works on swagger

timDeHof
timDeHof previously approved these changes Sep 1, 2024
View reviewed changes
Copy link
Contributor

@timDeHof timDeHof left a comment

Choose a reason for hiding this comment

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

All tests passed! everything works on swagger

@Ajen07 Ajen07 requested a review from timDeHof September 1, 2024 09:38
timDeHof
timDeHof previously requested changes Sep 6, 2024
View reviewed changes
Copy link
Contributor

@timDeHof timDeHof left a comment

Choose a reason for hiding this comment

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

I have added some comments to code.

@Ajen07 Ajen07 requested review from timDeHof and cherylli September 8, 2024 06:23
cherylli
cherylli reviewed Sep 9, 2024
View reviewed changes
Copy link
Contributor

@cherylli cherylli left a comment

Choose a reason for hiding this comment

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

We should use more descriptive commit message rather than minor changes

src/ability/conditions/meetingOrAgenda.ability.ts Outdated Show resolved Hide resolved
@Ajen07 Ajen07 requested a review from cherylli September 9, 2024 10:40
@Ajen07 Ajen07 marked this pull request as draft September 9, 2024 10:49
@Ajen07 Ajen07 force-pushed the feature/casl-permission-voyage-sprints branch from c900557 to ad2cc7c Compare September 9, 2024 10:56
@Ajen07 Ajen07 marked this pull request as ready for review September 9, 2024 10:57
@cherylli cherylli closed this Sep 9, 2024
@cherylli cherylli deleted the feature/casl-permission-voyage-sprints branch September 9, 2024 12:09
@cherylli cherylli restored the feature/casl-permission-voyage-sprints branch September 9, 2024 12:14
@cherylli cherylli reopened this Sep 9, 2024
@Ajen07 Ajen07 requested a review from cherylli September 9, 2024 14:13
cherylli
cherylli approved these changes Sep 10, 2024
View reviewed changes
Copy link
Contributor

@cherylli cherylli left a comment

Choose a reason for hiding this comment

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

looks good, ran all tests locally

@cherylli cherylli dismissed timDeHof’s stale review September 15, 2024 09:20

requested changes made

timDeHof
timDeHof approved these changes Sep 15, 2024
View reviewed changes
Copy link
Contributor

@timDeHof timDeHof left a comment

Choose a reason for hiding this comment

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

looks good, everything passes in the tests and on swagger.

timDeHof
timDeHof approved these changes Sep 15, 2024
View reviewed changes
Copy link
Contributor

@timDeHof timDeHof left a comment

Choose a reason for hiding this comment

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

looks good, everything passes in the tests and on swagger.

@cherylli cherylli removed the review label Sep 15, 2024
@Ajen07 Ajen07 merged commit 0445f6e into dev Sep 16, 2024
1 check passed
@Ajen07 Ajen07 deleted the feature/casl-permission-voyage-sprints branch September 16, 2024 03:56
@cherylli cherylli mentioned this pull request Sep 20, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cherylli cherylli cherylli approved these changes

@timDeHof timDeHof timDeHof approved these changes

Assignees

@timDeHof timDeHof

@Ajen07 Ajen07

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Ajen07 @cherylli @timDeHof