-
Notifications
You must be signed in to change notification settings - Fork 588
Improve service url builder for tenant perspective org urls #7565
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
base: master
Are you sure you want to change the base?
Conversation
| String accessingOrganization = | ||
| PrivilegedCarbonContext.getThreadLocalCarbonContext().getApplicationResidentOrganizationId(); |
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.
Log Improvement Suggestion No: 1
| String accessingOrganization = | |
| PrivilegedCarbonContext.getThreadLocalCarbonContext().getApplicationResidentOrganizationId(); | |
| String accessingOrganization = | |
| PrivilegedCarbonContext.getThreadLocalCarbonContext().getApplicationResidentOrganizationId(); | |
| if (log.isDebugEnabled()) { | |
| log.debug("Accessing organization ID: " + (StringUtils.isNotBlank(accessingOrganization) ? accessingOrganization : "None")); | |
| } |
| } else if (StringUtils.isNotBlank(accessingOrganization)) { | ||
| resolvedUrlStringBuilder.append("/t/").append(tenantDomain).append("/o/").append(accessingOrganization); | ||
| return; |
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.
Log Improvement Suggestion No: 2
| } else if (StringUtils.isNotBlank(accessingOrganization)) { | |
| resolvedUrlStringBuilder.append("/t/").append(tenantDomain).append("/o/").append(accessingOrganization); | |
| return; | |
| } else if (StringUtils.isNotBlank(accessingOrganization)) { | |
| resolvedUrlStringBuilder.append("/t/").append(tenantDomain).append("/o/").append(accessingOrganization); | |
| if (log.isDebugEnabled()) { | |
| log.debug("Building tenant and organization qualified URL for tenant: " + tenantDomain + " and organization: " + accessingOrganization); | |
| } | |
| return; |
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.
AI Agent Log Improvement Checklist
- The log-related comments and suggestions in this review were generated by an AI tool to assist with identifying potential improvements. Purpose of reviewing the code for log improvements is to improve the troubleshooting capabilities of our products.
- Please make sure to manually review and validate all suggestions before applying any changes. Not every code suggestion would make sense or add value to our purpose. Therefore, you have the freedom to decide which of the suggestions are helpful.
✅ Before merging this pull request:
- Review all AI-generated comments for accuracy and relevance.
- Complete and verify the table below. We need your feedback to measure the accuracy of these suggestions and the value they add. If you are rejecting a certain code suggestion, please mention the reason briefly in the suggestion for us to capture it.
| Comment | Accepted (Y/N) | Reason |
|---|---|---|
| #### Log Improvement Suggestion No: 1 | ||
| #### Log Improvement Suggestion No: 2 |
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.
Pull Request Overview
This PR enhances the service URL builder to support tenant perspective organization URLs by incorporating the accessing organization ID from the thread local context when constructing URLs.
Key Changes
- Added logic to retrieve the accessing organization ID from
PrivilegedCarbonContext - Updated URL construction to include organization qualifier when an accessing organization is present
- Modified the
setURLmethod signature to accept the accessing organization parameter
| } | ||
|
|
||
| private void setURL(StringBuilder resolvedUrlStringBuilder, String tenantDomain) { | ||
| private void setURL(StringBuilder resolvedUrlStringBuilder, String tenantDomain, String accessingOrganization) { |
Copilot
AI
Oct 20, 2025
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.
The method signature has been updated to include a new parameter 'accessingOrganization', but the method lacks a docstring explaining the purpose and usage of this parameter. Add a JavaDoc comment documenting all parameters, especially the newly added 'accessingOrganization' parameter.
|
Codecov Report❌ Patch coverage is
❌ Your patch check has failed because the patch coverage (28.57%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## master #7565 +/- ##
============================================
+ Coverage 50.85% 50.87% +0.02%
+ Complexity 19001 18999 -2
============================================
Files 2099 2099
Lines 121819 121952 +133
Branches 25296 25351 +55
============================================
+ Hits 61952 62047 +95
- Misses 51867 51894 +27
- Partials 8000 8011 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|



Proposed changes in this pull request
This PR adds the support to build tenant perspective org urls.