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

chore(CxMembership): Set Catena-X Membership in BPDM #1118

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

tfjanjua
Copy link
Contributor

@tfjanjua tfjanjua commented Oct 25, 2024

Description

Set Catena-X Membership in BPDM.

Why

As Portal, we would like to maintain a list of all companies that were onboarded to the Catena-X network. This membership information is meant to be stored in the BPDM Pool. We therefore need to update the isCatenaXMember flag for a BPNL once the company has been onboarded.

Issue

Ref: #1111

Config PR: eclipse-tractusx/portal#472

BPDM PR: eclipse-tractusx/bpdm#1072

Checklist

  • I have followed the contributing guidelines
  • 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

@tfjanjua tfjanjua self-assigned this Oct 25, 2024
@tfjanjua tfjanjua added the enhancement New feature or request label Oct 25, 2024
@tfjanjua tfjanjua force-pushed the chore/1111-Set-Catena-X-Membership-in-BPDM branch from 05892e0 to 508af80 Compare October 28, 2024 19:03
@tfjanjua tfjanjua marked this pull request as ready for review October 30, 2024 12:43
@ntruchsess ntruchsess linked an issue Nov 20, 2024 that may be closed by this pull request
4 tasks
@ntruchsess ntruchsess self-assigned this Nov 20, 2024
@ntruchsess ntruchsess force-pushed the chore/1111-Set-Catena-X-Membership-in-BPDM branch from 8a0304f to c649d4d Compare November 20, 2024 14:44
Copy link

sonarcloud bot commented Nov 20, 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.

@tfjanjua : I did fix a few findings and force-pushed to your repo after rebasing on latest main. Please pull he changes before proceeding. (And also have a look on what I changed to avoid such findings in the future).
PR currently cannot be merged as the configuration in portal-repo does not match. Also we need to come up with a better approach to handle multiple bpdm deployments without hardcoding the fact that this particular call is different into the service-method.


// Same User (which we have for BPDM service) can be used to call Business Partner Pool
// But BaseAddress of Business Partner Pool is different as its deployed on another server.
httpClient.BaseAddress = new Uri(_settings.BusinessPartnerPoolBaseAddress);
Copy link
Contributor

@ntruchsess ntruchsess Nov 20, 2024

Choose a reason for hiding this comment

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

(while technically working) this is not how a httpClientFactory (which is used in tokenService) should be used. The service-code should not have to deal with the baseAddress.

@tfjanjua
Copy link
Contributor Author

@tfjanjua : I did fix a few findings and force-pushed to your repo after rebasing on latest main. Please pull he changes before proceeding. (And also have a look on what I changed to avoid such findings in the future). PR currently cannot be merged as the configuration in portal-repo does not match. Also we need to come up with a better approach to handle multiple bpdm deployments without hardcoding the fact that this particular call is different into the service-method.

Thank you @ntruchsess , providing your feedback.

I've looked at your commit and found some formatting and addition of license related information (especially in migration files).

I'll have a look at configuration PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: IN PROGRESS
Development

Successfully merging this pull request may close these issues.

Process Worker | Set Catena-X Membership in BPDM
3 participants