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

[DEVSU-2468] add POST route to gkb user creation #409

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

kttkjl
Copy link
Member

@kttkjl kttkjl commented Nov 27, 2024

Separate graphkb user creation from ipr user creation, so everything is more in sync in the front-end

See DESVU-2468 for current blocker, which is not enough permissions on IPR API agent user

@kttkjl kttkjl self-assigned this Nov 27, 2024
@kttkjl kttkjl requested review from elewis2 and Nithriel November 27, 2024 22:20

This comment has been minimized.

@kttkjl kttkjl force-pushed the feat/DEVSU-2468-gkb-user-creation-separate branch from 031c5a6 to e18e361 Compare November 27, 2024 22:21
@codecov-commenter
Copy link

codecov-commenter commented Nov 27, 2024

Codecov Report

Attention: Patch coverage is 18.18182% with 9 lines in your changes missing coverage. Please review.

Project coverage is 76.85%. Comparing base (90e990b) to head (c82ee26).

Files with missing lines Patch % Lines
app/routes/graphkb/index.js 18.18% 8 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #409      +/-   ##
===========================================
- Coverage    76.96%   76.85%   -0.12%     
===========================================
  Files          179      179              
  Lines         6038     6048      +10     
  Branches       696      697       +1     
===========================================
+ Hits          4647     4648       +1     
- Misses        1314     1322       +8     
- Partials        77       78       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This comment has been minimized.

} catch (error) {
logger.error(error);
return res.status(StatusCodes.SERVICE_UNAVAILABLE).json({
message: 'GraphKB user creation error',
Copy link
Contributor

Choose a reason for hiding this comment

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

It'd be good to include the error in the reply so the user knows why its failing, agree @elewis2 ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yes makes sense to me

Copy link
Collaborator

Choose a reason for hiding this comment

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

(pinging @kttkjl )

Copy link
Member Author

Choose a reason for hiding this comment

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

would commit c82ee26 be better?

This comment has been minimized.

Copy link
Collaborator

@elewis2 elewis2 left a comment

Choose a reason for hiding this comment

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

see comment in conversation

This comment has been minimized.

Copy link

Unit Test Results

    1 files  ±0    62 suites  ±0   2m 44s ⏱️ ±0s
565 tests ±0  565 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
562 runs  ±0  562 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit c82ee26. ± Comparison against base commit 90e990b.

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.

4 participants