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(tech user): add internal invitation tech user #1002

Merged

Conversation

typecastcloud
Copy link
Contributor

@typecastcloud typecastcloud commented Sep 17, 2024

Description

Add Registration Internal as new technical user that can be created in portal.

New technical_roles_management composite role with name Registration Internal with:

  • invite_new_partner
  • view_submitted_applications

is also being added as new client role in eclipse-tractusx/portal-iam#189

Why

Operator company wants to integrate 3rd party service to send out invites. Allows inviting and reviewing status via API.

Issue

Refs: #932
Related: eclipse-tractusx/portal-iam#189

Checklist

Please delete options that are not relevant.

  • I have followed the contributing guidelines
  • I have performed IP checks for added or updated 3rd party libraries
  • I have created and linked IP issues or requested their creation by a committer
  • I have performed a self-review of my own code
  • I have successfully tested my changes locally
  • I have added tests that prove my changes work
  • I have checked that new and existing tests pass locally with my changes
  • I have commented my code, particularly in hard-to-understand areas

Copy link
Contributor

@ntruchsess ntruchsess left a comment

Choose a reason for hiding this comment

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

@typecastcloud technically this looks fine, but you do not declare to have tested the change (together with iam-data in eclipse-tractusx/portal-iam#189) at least locally?
(I can see the unit-tests in CI are failing, but that seems to be unrelated to your change. I'll look into that)

@typecastcloud typecastcloud marked this pull request as draft September 17, 2024 11:07
@typecastcloud
Copy link
Contributor Author

typecastcloud commented Sep 17, 2024

@typecastcloud technically this looks fine, but you do not declare to have tested the change (together with iam-data in eclipse-tractusx/portal-iam#189) at least locally? (I can see the unit-tests in CI are failing, but that seems to be unrelated to your change. I'll look into that)

Hi @ntruchsess, I converted the PRs back to draft until I have fully tested this. I only ran dotnet test until now and was trying to find the reason for the failing test in pipeline. I first requested initial review from internal developers to get help fully testing the changes.

Thank you for the quick reply.

@typecastcloud
Copy link
Contributor Author

@ntruchsess tests are all green now. I also used the wrong offer id in user_roles.json. I'll run everything locally once and check if correct client gets createn. After that I'll set to ready for review.

@typecastcloud typecastcloud marked this pull request as ready for review September 19, 2024 09:32
evegufy
evegufy previously approved these changes Sep 23, 2024
Phil91
Phil91 previously approved these changes Sep 24, 2024
@Phil91
Copy link
Member

Phil91 commented Sep 24, 2024

@typecastcloud could you please resolve the conflicts? In my opinion everything else looks good

Copy link
Contributor

@ntruchsess ntruchsess left a comment

Choose a reason for hiding this comment

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

@typecastcloud : please resolve the conflict in CHANGELOG.md by doing a rebase on latest main.

Allows adding technical user for invitation api as CX Operator.

Refs: eclipse-tractusx#932
Use "Technical User Management" instead of "Portal".
Technical User Management as offer id in Registration Internal requires updated tests.
@typecastcloud
Copy link
Contributor Author

@Phil91 @ntruchsess rebased and changelog is adjusted.

New line was missing at the end of the changelog too.

CHANGELOG.md Outdated Show resolved Hide resolved
evegufy
evegufy previously approved these changes Sep 24, 2024
Copy link
Contributor

@ntruchsess ntruchsess left a comment

Choose a reason for hiding this comment

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

@typecastcloud : unit-test GetServiceAccountRolesAsync_WithValidData_ReturnsExpected is failing. An additional assert has been added on main that needs to be adjusted in the PR (see my suggested change)

Co-authored-by: Norbert Truchsess <[email protected]>
Copy link

sonarcloud bot commented Sep 26, 2024

@ntruchsess ntruchsess merged commit 5f8b7fe into eclipse-tractusx:main Sep 26, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: USER READY
Development

Successfully merging this pull request may close these issues.

Operator | Send invites with custom technical user | Change of Access Policy
4 participants