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.

Sign up for GitHub

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

SLVS-1455 Make Id of type URL #5698

Merged
merged 12 commits into from
Sep 25, 2024
Merged

SLVS-1455 Make Id of type URL #5698

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
81a1e61
Change sonarcloud ServerConnection Id to Uri
vnaskos-sonar Sep 19, 2024
02dc1f9
Prevent leaking UI models to backend
vnaskos-sonar Sep 20, 2024
553831e
Continue on separation of concerns
vnaskos-sonar Sep 24, 2024
db71335
Replace hardcoded url with Resource
vnaskos-sonar Sep 23, 2024
e0b7b61
Fix rebase issues
vnaskos-sonar Sep 24, 2024
db52a0c
Validation of new SonarCloud connection does not have an organization
vnaskos-sonar Sep 24, 2024
f4b868e
Throw exception if credentialsModel is null
vnaskos-sonar Sep 24, 2024
955edfe
Throw exception in case of unknown connection type
vnaskos-sonar Sep 24, 2024
290d104
Reuse CoreStrings.SonarCloudUrl
vnaskos-sonar Sep 25, 2024
734aa31
Add documentation
vnaskos-sonar Sep 25, 2024
56b85a7
Fix test
vnaskos-sonar Sep 25, 2024
6f9ed50
Replace redundant check with null conditional operator
vnaskos-sonar Sep 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion src/ConnectedMode/SlCoreConnectionAdapter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,11 @@ public Task<AdapterResponseWithData<List<OrganizationDisplay>>> GetOrganizations

try
{
var credentials = MapCredentials(credentialsModel?.ToICredentials());
if (credentialsModel == null)
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need this explicit check. If it is null, there will be a null reference exception that will be caught by the catch block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In case the credentialsModel is null the ToICredentials will throw a generic null exception. In this case I believe it's better to be proactive and add back the .? so the exception we get becomes specific to the lack of credentials.

{
throw new ArgumentException($"Unexpected {nameof(ICredentialsModel)} argument");
}
var credentials = MapCredentials(credentialsModel.ToICredentials());
var response = await connectionConfigurationSlCoreService.ListUserOrganizationsAsync(new ListUserOrganizationsParams(credentials));
var organizationDisplays = response.userOrganizations.Select(o => new OrganizationDisplay(o.key, o.name)).ToList();

Expand Down