Skip to content

Conversation

@sccalabr
Copy link

@sccalabr sccalabr commented Oct 10, 2025

@CLAassistant
Copy link

CLAassistant commented Oct 10, 2025

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ sccalabr
❌ totesforlife
You have signed the CLA already but the status is still pending? Let us recheck it.

<FormattedMessage
id='Apis.Details.Configuration.components.storeVisibility.tooltip.private.desc'
defaultMessage={
'The {type} is visible only to users in the current tenant domain.'
Copy link
Member

Choose a reason for hiding this comment

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

The word current tenant domain might not be intuitive. Shall we use similar wording in our official documentation for this option?

@sccalabr
Copy link
Author

Let me know if that is what you were thinking

@tharikaGitHub
Copy link
Member

Hi @sccalabr, Thank you for working on the requested changes. I applied your fix locally and the issue is fixed. During the application of the fix, several EsLint related issues were encountered. Shall we run an npm run build:prod build and verify EsLint issues and correct them?

@tharikaGitHub
Copy link
Member

Hi @sccalabr, any update on the above?

@sccalabr
Copy link
Author

I'I'll get to it on the weekend :)

"integrity": "sha512-UlLAnTPrFdNGoFtbSXwcGFQBtQZJCNjaN6hQNP3UPvuNXT1i82N26KL3dZeIpNalWywr9IuQuncaAfUaS1g6sQ==",
"devOptional": true,
"license": "MIT",
"peer": true,
Copy link
Member

Choose a reason for hiding this comment

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

We will not need changes done to this file @sccalabr.

"Apis.Details.Configuration.components.Social.slack": "Slack URL",
"Apis.Details.Configuration.components.Social.slack_url.help": "This Slack Channel URL will be available in the {type} overview page in developer portal",
"Apis.Details.Configuration.components.StoreVisibility.dropdown.public": "Public",
"Apis.Details.Configuration.components.StoreVisibility.tooltip.private": "Visible to my domain :",
Copy link
Member

Choose a reason for hiding this comment

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

Shall we keep line 616 and 617 only, which contains your changes and remove the rest of the changes?

@tharikaGitHub
Copy link
Member

Hi @sccalabr did you get a chance to address the new review comments?

@sccalabr
Copy link
Author

That's the output from running the command. I think we should keep it do the whole file is formatted correctly

@tharikaGitHub
Copy link
Member

Hi @sccalabr,

After building the apim-apps repository using the mvn clean install command with node version v22.x, I do not see any changes applied to package-lock.json file and also the en.json does not contain changes apart from the new ids relevant to your fix. What are the steps you followed to build the repo?

And also your changes in the file StoreVisibility.jsx do not compile correctly as they exceed the 120 character length.

Shall we fix this?

@sccalabr
Copy link
Author

I ran npm run build:prod

@tharikaGitHub
Copy link
Member

Hi @sccalabr

Okay. Can you try with mvn clean install after removing the changes from files en.json and package-lock.json? Remember to fetch the latest changes from upstream main branch before building using mvn clean install. You can see that our automated repo build has also failed with the changes in this PR. So shall we correct it and update the PR? I will be able to approve the PR after that because the changes you have done to the StoreVisibility.jsx file are correct.

Thanks,
Tharika.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 2, 2025

@tharikaGitHub
Copy link
Member

Hi @sccalabr Thank you for addressing the comments. Shall we fetch from upstream repo and try building again with mvn clean install as mentioned in comment [1] because I see there are unrelated changes in the en.json file in the PR. Make sure to fetch the latest changes.

[1] #1169 (comment)

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