-
Notifications
You must be signed in to change notification settings - Fork 39
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
[Reputation Oracle] NDA #3132
base: develop
Are you sure you want to change the base?
[Reputation Oracle] NDA #3132
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
@flopez7 please, rebase develop branch to resolve conflicts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good in general
Left some suggestions
packages/apps/reputation-oracle/server/src/modules/nda/nda.module.ts
Outdated
Show resolved
Hide resolved
packages/apps/reputation-oracle/server/src/modules/nda/nda.controller.ts
Show resolved
Hide resolved
packages/apps/reputation-oracle/server/src/modules/nda/nda.controller.ts
Outdated
Show resolved
Hide resolved
packages/apps/reputation-oracle/server/src/modules/nda/nda.controller.ts
Outdated
Show resolved
Hide resolved
packages/apps/reputation-oracle/server/src/modules/nda/nda.service.spec.ts
Outdated
Show resolved
Hide resolved
packages/apps/reputation-oracle/server/src/modules/nda/nda.service.spec.ts
Outdated
Show resolved
Hide resolved
packages/apps/reputation-oracle/server/src/modules/nda/nda.service.ts
Outdated
Show resolved
Hide resolved
packages/apps/reputation-oracle/server/src/modules/user/user.entity.ts
Outdated
Show resolved
Hide resolved
packages/apps/reputation-oracle/server/src/modules/nda/nda.service.spec.ts
Outdated
Show resolved
Hide resolved
* feat: add NDA module with service, controller, and related functionality * feat: add HTTP status code to signNDA endpoint and update tests to use ndaServiceMock * Move user within the signNDA tests as it is not being modified
packages/apps/human-app/server/src/modules/nda/spec/nda.fixtures.ts
Dismissed
Show dismissed
Hide dismissed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to verify that the nda is signed to allow accessing to oracles and jobs
@portuu3 I think that it should be done together with the frontend implementation. However, we can implement this with a feature flag on human app server. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
|
||
describe('signNDA', () => { | ||
let user: Pick<UserEntity, 'id' | 'email' | 'ndaSignedUrl'>; | ||
beforeEach(async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
beforeEach(async () => { | |
beforeEach(() => { |
Issue tracking
#3105
Context behind the change
add NDA module and related functionality
How has this been tested?
Launched locally.
Created one new user.
Signed the NDA
Release plan
Add
NDA_URL
in Reputation Oracle environmentPotential risks; What to monitor; Rollback plan
None