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: conflict detection support deploy on save files under multiple directories #3393

Merged
merged 14 commits into from
Jul 21, 2021

Conversation

violetyao
Copy link
Contributor

What does this PR do?

This PR addresses the issue that the new conflict detection mechanism does not work for deploy on save multiple files under multiple directories.

What issues does this PR fix or reference?

@W-9549835@

Functionality Before

We used relative paths in diffComponents to match local and remote components. This became problematic when we deploy on save multiple files under multiple directories: keys of cacheIndex are of format test1.cls while keys of projectIndex are of format folder1/test1.cls. Thus, when we assign cacheIndex.get(key) to cachePath, the value would be undefined.

Functionality After

We now use diffComponents to match local and remote components. We also have localRelPath and remoteRelPath for visualizing file diffs.

@violetyao violetyao requested a review from a team as a code owner July 7, 2021 20:16
@violetyao violetyao requested a review from xyc July 7, 2021 20:16
@violetyao violetyao changed the title Violet/conflict detection multi dir fix: conflict detection support deploy on save files under multiple directories Jul 7, 2021
@AnanyaJha
Copy link
Collaborator

@violetyao quick question for you, how have you been testing these changes? I haven't been able to view multiple conflicts from deploy on save yet so I'd like to make sure I'm reproducing the issue correctly 😄
My current strategy has involved deploying three classes to the org outside of VS Code, making changes to those three classes within VS Code, and saving the classes within VS Code. At this point, the conflict detection modal appears, but I'm only seeing a conflict in one of the classes rather than all three

@violetyao
Copy link
Contributor Author

violetyao commented Jul 9, 2021

@violetyao quick question for you, how have you been testing these changes? I haven't been able to view multiple conflicts from deploy on save yet so I'd like to make sure I'm reproducing the issue correctly 😄
My current strategy has involved deploying three classes to the org outside of VS Code, making changes to those three classes within VS Code, and saving the classes within VS Code. At this point, the conflict detection modal appears, but I'm only seeing a conflict in one of the classes rather than all three

Hi Ananya! Have you modified the three classes in their org web page after deploying them? The remote last modified date and local last sync date would be the same if we don't modify the classes in their org web page after deployment. We can also talk about this in our sync today! I usually 1) deploy three classes 2) make edits to them in remote org web page 3) make edits in local vscode 3) deploy on save (using command + option + s) in local vscode and view conflicts.
Screen Shot 2021-07-09 at 9 14 59 AM

@maxcamp
Copy link
Contributor

maxcamp commented Jul 15, 2021

While these changes are a great start to fixing the multiple directories problem, I found that using path.basename() leads to problems with metadata types that do not follow the "matchingContent" and "bundle" strategies. Examples include StaticResources ("mixedContent" strategy) and CustomObjects ("decomposed" strategy). I'm going to push some changes soon that hopefully fix those issues.

@maxcamp
Copy link
Contributor

maxcamp commented Jul 15, 2021

@AnanyaJha
Copy link
Collaborator

Looks good to me :shipit:

@maxcamp maxcamp merged commit 47976f8 into develop Jul 21, 2021
@maxcamp maxcamp deleted the violet/conflict-detection-multi-dir branch July 21, 2021 22:41
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.

3 participants