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

Conflict Detection - Next iteration - Spec for feedback #2034

Closed
smaddox-sf opened this issue Feb 25, 2020 · 35 comments
Closed

Conflict Detection - Next iteration - Spec for feedback #2034

smaddox-sf opened this issue Feb 25, 2020 · 35 comments
Labels

Comments

@smaddox-sf
Copy link
Contributor

smaddox-sf commented Feb 25, 2020

I’m posting the details of our next iteration on the Conflict Detection feature. Check out the plans and leave a comment if you have input

Problem we are seeking to solve:
When a user deploys metadata to the org, VS Code should check to ensure that the local metadata changes won't silently overwrite changes on the org. This only applies to orgs without source tracking (such as sandboxes).

Current beta:
The open beta for the first iteration of this feature is available. Check it out and see the details in our documentation. With this version, you can choose to detect differences for manifests only. When a difference is detected between the Org & local project, you see a list of files with differences in the Output panel. If you want to override, you are given the command to execute. This is strictly doing a diff between your local and the org. The beta applied to both deploy & retrieve with a manifest.

Vision for the next iteration:
We will expand to all Deploy commands and add some intelligence to do more than just a diff. To determine potential conflicts, we will look at the last modified date in the Org and the date you last synced the local file. If the date is more recent on the file in the org, we will include this as a potential conflict. When any potential is detected, you have an option to view the details or overwrite immediately. If you choose the ‘Overwrite’ option, it will be the same as forcing the deployment via the CLI sfdx force:source:deploy ... --force.

Illustration of the new dialog that appears when differences are detected:
image

If you choose to ‘Show Conflicts’, you will see the list of conflicted files via a temporary tab on the side panel. You can click each file to see a diff between the local version and the org version at the time the deploy was executed. While viewing the diff, you can edit the local version.

Note: This view will not persist if you close the project or close VS Code. (However, you can see the view again by running another deploy command or a Diff command.)

Illustration of the potential view of conflicted files:
image

After you review the potential conflicts, it's up to you to continue with the deploy and force an overwrite or perhaps take other actions to resolve conflicts.

This feature is controlled by a VS Code setting, ‘Detect Conflicts at Sync’. That setting is off by default.

If you want to check for differences before a retrieve, use Source Diff.

@abd3
Copy link

abd3 commented Mar 9, 2020

It might be helpful if there was an affordance for each file, like a button allowing you to "keep local version" or "keep remote version". You should also be explicit in each pane, whether it's the local version or the remote version if possible; that can get confusing for users.

This is a somewhat separate issue, but it's a major challenge that the metadata API allows users to retrieve or deploy metadata that's part of an unlocked package without giving any warning that you're modifying the contents of the package. If there was a way to flag or warn if you were overwriting the contents of an unlocked package, that could help users keep the integrity of the packages. They should make those changes in the package, not directly in the org.

@smaddox-sf
Copy link
Contributor Author

Thanks for the feedback @abd3! It's useful to know what would help your team. Regarding labeling the panels, it's a bit small in the screenshot above, but the titlebar has the org alias prefixed on the left and 'local' prefixed on the right. Perhaps this isn't discoverable enough?

The input regarding packages is interesting. We'll consider that area in the future.

@abd3
Copy link

abd3 commented Mar 10, 2020

I see it now that I'm looking at the image in fullscreen. My intuition says that the local copy should be on the left, with the idea that your goal is to send to the target org on the right (left to right flow), but others may think differently

@smaddox-sf smaddox-sf changed the title Conflict Detection - Spec for next iteration for feedback Diff Detection (aka Conflict Detection) - Spec for next iteration for feedback May 29, 2020
@smaddox-sf smaddox-sf added Spec Details of a feature for feedback and removed type:feedback Feedback for new features labels May 29, 2020
@digiur
Copy link

digiur commented Jun 2, 2020

Does this work for all metadata files?

@smaddox-sf
Copy link
Contributor Author

@digiur - Yes, will work for all files you Deploy & Retrieve

@amnezhdez
Copy link

Thank you for the job, it's a very useful functionality.

It might be helpful if there was a configuration, to select some changes to ignore them. For example, ignore conflicts in minor changes, such as description or help text.

@AmrutHunashyal
Copy link

Hi @smaddox-sf can you please let us know when Diff Detection would be available (even beta) for Deploy & Retrieve source to/from org. thanks.

@smaddox-sf
Copy link
Contributor Author

Hi @AmrutHunashyal - I'm glad you're looking forward to this feature! Our goal is to have it ready for all deploy / retrieve commands within about 6 months. Feel free to check out our roadmap to see what we're working on.

@DaniyelShchuritz
Copy link

DaniyelShchuritz commented Sep 26, 2020

Hi @smaddox-sf, this is still not working for me.

  1. I've enabled the Detect Conflicts At Sync in my VS Code
  2. Change the apex class directly on the org and save it
  3. Change the same class locally with my VS Code
  4. Run this command
    sfdx force:source:deploy -x manifest/Sprint.xml -u myOrg --verbose

And it deploys successfully and overrides the changes I created directly on the org without mentioning any conflicts.
Am I doing something wrong?

Many thanks!

@smaddox-sf
Copy link
Contributor Author

HI @DaniyelShchuritz - Can you open a new issue (type: Bug) with the details so we can check into it? This "issue" is the spec for feedback and it's easier to discuss 1 topic per issue versus mixing them.

@DaniyelShchuritz
Copy link

DaniyelShchuritz commented Oct 7, 2020 via email

@AmrutHunashyal
Copy link

@smaddox-sf

[Hi @AmrutHunashyal - I'm glad you're looking forward to this feature! Our goal is to have it ready for all deploy / retrieve commands within about 6 months.]

It's been 6 months, any update on this ?

@smaddox-sf
Copy link
Contributor Author

Hi @AmrutHunashyal - Thanks for checking back. We've completed the foundational libraries needed to make this feature performant. Things have taken longer than originally thought. This is still planned as near-term work.

@smaddox-sf smaddox-sf changed the title Diff Detection (aka Conflict Detection) - Spec for next iteration for feedback Diff Detection (aka Conflict Detection) - Spec feedback Mar 1, 2021
@smaddox-sf smaddox-sf changed the title Diff Detection (aka Conflict Detection) - Spec feedback Diff Detection (aka Conflict Detection) - Spec for feedback Mar 1, 2021
@chasd00
Copy link

chasd00 commented Mar 24, 2021

Thanks for working on this, i've been fighting code overwrite problems in my teams and finally found out some users are using SFDX commands for deployment in VSCode while others are using forcecode deploy-on-save (which gives a warning). Any update on when the next iteration will be released? I'm very interested in this feature applied to non-manifest deployments.

@ralphcallaway
Copy link

in my experience most people are using the deploy on save functionality

"salesforcedx-vscode-core.push-or-deploy-on-save.enabled": true

if that's not covered by the current version, i think that's a must

and if it is covered by the current version please let me know, because it's not working, and i need to file a bug

@chasd00
Copy link

chasd00 commented Apr 2, 2021

Can we get an update on when the next iteration can be expected? I have a team of 25 devs and leadership+client is getting pretty upset when i have to give yet another update "...we're dealing with a code overwrite issue.." heh

@smaddox-sf
Copy link
Contributor Author

Hi @chasd00 - Thanks for checking back on this. I'm sorry your team is continuing to have overwrite issues. We will start working on the completion of this feature later this month. We haven't done planning yet, so I don't have an ETA on release just yet.

@ralphcallaway - Question for you on deploy-on-save - Right now, Diff Detection doesn't have much intelligence. It is simply telling you any difference between what you are deploying & the org (including your own change). This means it would prompt you each time you save, even if no one else has changed the file. Do you think it would still be helpful to your team in that state?

@ChuckJonas
Copy link

ChuckJonas commented Apr 2, 2021

@smaddox-sf having it just diff local vs remote won't help anything (as you already pointed out).

You need to track the Salesforce ApexClass.LastModifiedDate locally in the project:

MyApexClass -> 1601332000000,
MyObject__c -> 1601532000000

This table would get updated anytime you save or retrieve code.

When you go to deploy metadata, you would:

  1. Grab the Local Timestamp from the table & the content from the editor
  2. Query the current Timestamp & Content from the target org
  3. Compare Timestamps -> Compare Code -> Display warning
local = getTimestampAndContent()
remote = [SELECT LastModifiedDate, Content FROM ApexClass Where ID = :local.Id];
IF  local.Timestamp < current.Timestamp THEN 
   diff_Count = Diff_Files(local.Content, remote.Content);
   IF diff_Count > 0 THEN Display_Warning()

For reference, this is how mavensmate did it. Basically the solution above, with an additional "dirty" flag.

@ralphcallaway
Copy link

ralphcallaway commented Apr 2, 2021

@smaddox-sf @ChuckJonas that's a great way to go, one suggestion

  • don't update the timestamp on save
  • whenever a remote save (i.e. deployment, creation) succeeds, retrieve the resulting metadata
  • whenever you retrieve metadata, update the local timestamp with the remote timestamp

@ralphcallaway
Copy link

beyond making sure the timestamps are exact and you don't have to think about thresholds for save time, it has the added benefit of ensuring the metadata in your editor matches what's on salesforce exactly

@ralphcallaway
Copy link

@smaddox-sf this would need to happen for every save route in vscode, so you'd want to hook in at the lowest level possible

from there, you'll need 1) a force option, and 2) a diff viewer to merge conflicts (nice to have if you're time strapped)

@smaddox-sf smaddox-sf changed the title Diff Detection (aka Conflict Detection) - Spec for feedback Conflict Detection - Next iteration - Spec for feedback May 3, 2021
@AmrutHunashyal
Copy link

@smaddox-sf any idea when this will be rolled out ?

@ralphcallaway
Copy link

ralphcallaway commented Jul 6, 2021

@smaddox-sf any updates on if the next iteration will include any of our suggestions? as it's described the proposed version isn't going to be helpful. i run a consultancy and it's super embarassing when you have your team repeatedly overwrite someone elses work and then i have to explain to the team member that while everything other salesforce IDE to date has had conflict detection, vs code doesn't in any useful way, and you need to remember to retrieve every file you edit first to make sure you don't overwrite anything ...

just wondering out loud, given there is an existing implementation of exactly this features that is under 100 lines and is performant, any reason salesforce isn't just adopting that? if there are any IP issues, I know joe ferraro the creator of mavensmate would surely adjust any licensing needed (the current version allows commercial use and modification, but does require attribution and same license which might be an issue)

@abd3
Copy link

abd3 commented Jul 6, 2021

In support of @ralphcallaway's suggestion, he and Joe Ferraro we're core developers on Mavensmate. They had this functionality five years ago, it's open source, and it worked great.

@smaddox-sf
Copy link
Contributor Author

Hi @ChuckJonas , @abd3 , @ralphcallaway and @AmrutHunashyal - Thanks for checking back and for your interest in this feature. I'm excited to say that the feature is nearly complete and we plan to move it to GA later this summer!! We're continuing to ship incremental progress weekly and I will post an update here when it is complete. Here's the updates we've made since I posted the original spec (original post updated to reflect reality as well):

  • A conflict is detected when the last modified date in Org is more recent than the date you last synced this file locally. (The local file could be synced either via a deploy or retrieve)
  • When you enable the 'Detect Conflicts at Sync' setting, we will check for conflicts on all Deploy operations (including deploy-on-save)
  • @abd3 - To start, we will surface the list of files with a potential conflict and you can see a Diff on each. Options from there will be to choose to do retrieve(s) or rerun the deploy with an overwrite. In the future, we may consider adding further sophistication on the resolution after potential conflicts are surfaced.

@monirkohi2006
Copy link

Doesn't look like the sync is happening at least on deploy. I get a conflict every time I make a change to the file and deploy.

@ralphcallaway
Copy link

@smaddox-sf just a thought, but is the "Sync" part of "Detect Conflicts at Sync" a little bit off? It seems like "Detect Conflicts on Save/Deploy" would be more appropriate.

@smaddox-sf
Copy link
Contributor Author

Thank you for participating in this discussion! We have delivered this feature so closing this issue. Check out the details in our documentation and give it a try. We always want to hear your feedback, so please log an issue if you have feedback or spot any problems.

@ralphcallaway
Copy link

@smaddox-sf excited to see progress, but i think you might want to roll this one back.

as it's implemented this would be better described as a save confirmation dialog. with it on, every save that changes anything (which is pretty much all of them, since what's the point of saving something that's the same as what's on the server), regardless of whether there have been changes on the server or not, sf generates this error message.

as analogy, you open a document, make some changes, save. oh wait conflict? oh i guess the version i started is different with than what i have now, but isn't that the whole point? I opened it with the intention of making changes, why would the system be surprised and complain about me making changes?

here's a little video i put together https://youtu.be/Dv95mF22qX8

my $.02, disable this feature asap, as it stands anyone who has this on is going to immediately hate due to the annoying number of pop ups mentioning conflicts that aren't really conflicts at all

as mentioned previously, there are several models, i.e. mm, that have implemented this successful for salesforce projects. i'd highly suggest starting with their setup as a design

@smaddox-sf
Copy link
Contributor Author

@ralphcallaway - Thanks for the feedback. Have you checked out the latest version of conflict detection? I agree that the Diff Detection wasn't very helpful - it's noise every time you deploy, which still makes you think and can lead to mistakes. With the updates to this feature, the logic now compares the modified date in the org with the local sync date to surface potential conflicts on deploy. You should not be seeing a conflict every time you deploy (unless there have been changes in the org). You can see more details in the documentation.

@ralphcallaway
Copy link

@smaddox-sf 😬 it doesn't sound like I'm on the latest version based on your description. Thanks for the clarification. I'll log a separate issue if it's still not working as designed.

@Yerrak
Copy link

Yerrak commented Nov 22, 2021

@ralphcallaway
Did you by any chance create that issue or resolved your problem? (How?)
I seem to be having the same problem. The last-modified condition as stated in the docs does not seem to be checked (correctly). Nearly every local change I try to deploy is detected as a conflict.

@taylorkrause
Copy link

@ralphcallaway
Not working for me as well. Version 53.4.2 of the Apex Extension.

@ralphcallaway
Copy link

@Yerrak see #3522, it's still open not working yet. They're making some assumptions about last modified dates that means if a compile takes to a bit to happen it looks like it's been modified on the server (just a few seconds later), so this will happen sometimes but not all the time. Kind of sad to see such a great feature get released with such a glaring bug that would occur with even a minimal amount of testing. I'm grateful smaddox-sf is monitoring at least.

@adayIvey
Copy link

I have noticed that if I do a deploy to the sandbox from vscode and then make a few more changes again and try to do a deploy when I know I am the only one making changes I get a conflict detection error the only way to get around this is to do a retrieve right after I have deployed. Is there anyway to just make the retrieve happen automatically after doing a deploy to stop getting a false conflict detection error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests