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

fix: Build absolute filePath so problems tab links to error location correctly #4246

Merged
merged 8 commits into from
Jun 30, 2022

Conversation

klewis-sfdc
Copy link
Contributor

What does this PR do?

A follow up to PR #4241 to fix the fileUri when constructed after a deployment error.

What issues does this PR fix or reference?

##4228, @W-11350093@

@klewis-sfdc klewis-sfdc requested a review from a team as a code owner June 29, 2022 22:41
const filePath = undefined;
const asoluteFilePath = getAbsoluteFilePath(filePath, workspacePath);
expect(asoluteFilePath).to.equal(workspacePath);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

I checked out the failing test and I'd just update it to the following:

 const [fileDesc, reportedError] = setDiagnosticsStub.getCall(
            index
          ).args;
          const expectedFile = vscode.Uri.file(
            getAbsoluteFilePath(row.filePath, getRootWorkspacePath())
          );
          expect(fileDesc).to.deep.equal(expectedFile);
          expect(reportedError).to.deep.equal([
            {
              message: row.error,
              range: new vscode.Range(
                row.lineNumber - 1,
                row.columnNumber - 1,
                row.lineNumber - 1,
                row.columnNumber - 1
              ),
              severity: vscode.DiagnosticSeverity.Error,
              source: row.type
            }
          ]);

lines 457-476

It separates the the assertions so we don't have to run it to know which on is failing. A very uningration solution as it pulls the utility into the test, but it was the best path I could come up with.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the var names in this method to more closely reflect the params of the method where they are defined.

Copy link
Contributor

@gbockus-sf gbockus-sf left a comment

Choose a reason for hiding this comment

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

Passed my QE:
✅ Verified navigation from problem to file when file is open
✅ Verified navigation from problem to file when file is closed
✅ Verified navigation from problem to file when file is open but not the active editor
✅ Verified when problem is generated during active editing
✅ Verified navigation from problem when the problem is reported after deploy from navigation
✅ Verified navigation from problem to file deploy via Deploy this source to Org

:shipit:

klewis-sfdc and others added 6 commits June 30, 2022 09:16
absolute file path is needed to make linking to the problem location work in the problems tab
can this function be refactored/moved to some type of shared file service module?
…esforcedx-vscode into ken/fix-error-file-uri-2
@@ -511,7 +514,10 @@ describe('Base Deploy Retrieve Commands', () => {
this.retrieveStub = sb
.stub(this.components, 'retrieve')
.returns({ pollStatus: this.pollStatusStub });
this.cacheSpy = sb.spy(PersistentStorageService.getInstance(), 'setPropertiesForFilesRetrieve');
this.cacheSpy = sb.spy(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some auto-formatting refactoring in this file.

Copy link
Contributor

@gbockus-sf gbockus-sf left a comment

Choose a reason for hiding this comment

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

Great! 🚢

@klewis-sfdc klewis-sfdc merged commit 7962499 into develop Jun 30, 2022
@klewis-sfdc klewis-sfdc deleted the ken/fix-error-file-uri-2 branch June 30, 2022 20:21
angela-young pushed a commit that referenced this pull request Jul 7, 2022
…correctly (#4246)

* fix: build absolute filePath so problems tab links to error correctly

* fix: export new function and add tests

* fix: update test to expect absolute file path

absolute file path is needed to make linking to the problem location work in the problems tab

* fix: default to the root workspace path

* fix: add todo

can this function be refactored/moved to some type of shared file service module?

* refactor: update var names to match parameter names of DiagnosticCollection#set
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.

None yet

2 participants