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

Allow reviewing changes before/after merging #1152

Open
phanimahesh opened this issue Oct 30, 2017 · 15 comments · May be fixed by #10173
Open

Allow reviewing changes before/after merging #1152

phanimahesh opened this issue Oct 30, 2017 · 15 comments · May be fixed by #10173
Assignees
Milestone

Comments

@phanimahesh
Copy link

phanimahesh commented Oct 30, 2017

Expected Behavior

I would like to see exactly what will happen when I'm merging databases. Added entries, removed/moved entries, modified entries. Just the path to entry (group/.../title), modification time and updated fields should be good enough.

Current Behavior

There is no feedback on changes happening during a merge.

Possible Solution

When merging, add an option to review and merge, which shows a summary of changes? Users who don't need this can use the other option, merge to skip reviewing.

Context

Each of my devices uses its own local copy and periodically I do a merge into and with a shared and synced copy, to avoid issues when sharing a database. If all devices operate on a shared version, and some devices have intermittent network connectivity or don't sync all the time it is possible to lose data since last update wins when syncing.

I'm worried about accidentally merging the wrong version and "losing" an update (not really sure if it is a real issue, given that entries track their history). Being able to review the diff will also tell me quickly if an entry I'm looking for is included in this merge, without having to merge everything.

Debug Info

eePassXC - Version 2.2.2
Revision: 6d46717

Libraries:

  • Qt 5.9.2
  • libgcrypt 1.8.1

Operating system: Arch Linux
CPU architecture: x86_64
Kernel: linux 4.13.7-1-ARCH

Enabled extensions:

  • KeePassHTTP
  • Auto-Type
@ghostsquad
Copy link

I recently found this tool: https://github.com/Narigo/keepass-diff

It would be really nice if a diff could be done. Even better if it was interactive like git, such that I could pick and choose what changes to keep.

@erik78se
Copy link

erik78se commented Sep 2, 2022

I too would like to see this feature as I'm constantly worried my entries are getting nuked by a merge.

@droidmonkey
Copy link
Member

Worry not! The original entry state is pushed into the entry's history. You can always revert to the previous version if needed. Make sure history is enabled for your database in the database settings.

@erik78se
Copy link

erik78se commented Sep 2, 2022

I don't think we mean the same thing.

I'm looking for knowing what entries have been changed as part of a merge. Today - its just a message something like "Merged successfully". But I wouldn't know anything about "WHAT" has been changed. It leaves me in a state of worry about me possibly have merged something I didn't like to or some accidental change which would cause havoc.

A list of net changes to a database merge from old to new would be very helpful. It surprises me every time that I can't find that feature when I'm looking for it in the menus.

@ghostsquad
Copy link

100% what @erik78se wrote. Iteration 1: show me the diff. Iteration 2: act like git and let me choose what changes I'd like to persist.

@schnerring
Copy link

Could we at least log the merge operations somewhere?

I use Syncthing to sync my database between my workstation and phone. I'm aware of the limitations of this approach, but since my password DB is never mutated from both the workstation and phone simultaneously, it's fine. I have been doing it like this for many years.

Merge conflicts sometimes occur when I forget to connect my phone via WiFi for a while (days to weeks). I occasionally check for Syncthing merge conflicts, and if I find any, I merge the out-of-sync databases. However, it's hard to remember what I've changed when the merge conflict is already 2 weeks old. My DB is pretty big, so manually looking through histories is also unfeasible.

I'd love a merge tool, but I'd already be happy if the success dialog box would at least give me a text log of what's been changed.

@droidmonkey
Copy link
Member

We offer that through the CLI tool right now, so if you have a major merge you can do that with the cli and see the log output.

You can also, post merge, search your database for * and sort by last modified to see which entries changed.

I absolutely want to introduce a merge log and conflict manager to v2.8.x.

@taminob
Copy link

taminob commented Jan 4, 2024

I'd be interested in working on this issue since I also run into this issue quite often and I currently manually merge databases by sorting by modification time.

As an MVP I'd propose a simple dialog listing all new/modified entries by group/title and a button to confirm or abort the merge.
In further iterations this could be refined into a separate list for modified entries showing the actual difference (e.g. in the style of the history view), the possibility to exclude certain entries or display "similar items" with e.g. same/similar group+title but different UUID.
In the end, this could then also be extended to offer conflict resolution.

I already had a look at the code base and it should be possible to add a dry_run flag to Merger::merge() in src/core/Merger.cpp to get the necessary changelist without immediately applying the changes.
The merge is done in DatabaseWidget::mergeDatabase() in src/gui/DatabaseWidget.cpp where it would be possible to display the dialog.

@droidmonkey
Copy link
Member

I'm very open to an mvp for this. Keep it simple.

@taminob taminob linked a pull request Jan 10, 2024 that will close this issue
@MathiasLui
Copy link

I just changed a specific password a few (maybe 5) times on my phone in DX, on my already opened Desktop, which is synced via syncthing it asked me 5 times if I wanted to merge the changes and I clicked "yes" every time. The password that was there in the end, was actually the very first one, before any of the 5 changes...

@taminob
Copy link

taminob commented Feb 28, 2024

I just changed a specific password a few (maybe 5) times on my phone in DX, on my already opened Desktop, which is synced via syncthing it asked me 5 times if I wanted to merge the changes and I clicked "yes" every time. The password that was there in the end, was actually the very first one, before any of the 5 changes...

Thanks for testing it that thoroughly, looks like there is some work to do. :)
I'll need to rewrite the PR in the next days anyways and will try a different approach - but that approach will probably disable auto-reload while merging and it will still only keep the password that was in the database when initiating the merge (although it will also ask only once for the merge). Is that the behavior you would expect? Or should a changed database on disk abort the merge?

@MathiasLui
Copy link

Thanks for testing it that thoroughly, looks like there is some work to do. :) I'll need to rewrite the PR in the next days anyways and will try a different approach - but that approach will probably disable auto-reload while merging and it will still only keep the password that was in the database when initiating the merge (although it will also ask only once for the merge). Is that the behavior you would expect? Or should a changed database on disk abort the merge?

Actually - I have used the newest release build, it looks like you're assuming I have tested the PR, but I didn't notice it exists until now, and blindly commented, sorry 😅

I feel like a database change on disk during a merge of a previous change should add that new change to the merges to be resolved - is that the behaviour that was added? Does it also preview e.g. the password to be compared?

@droidmonkey
Copy link
Member

Have you checked that entry's history?

@taminob
Copy link

taminob commented Feb 29, 2024

Actually - I have used the newest release build, it looks like you're assuming I have tested the PR, but I didn't notice it exists until now, and blindly commented, sorry 😅

Ah, I thought your "clicking yes" meant the confirmation dialog. :)

I feel like a database change on disk during a merge of a previous change should add that new change to the merges to be resolved - is that the behaviour that was added? Does it also preview e.g. the password to be compared?

The exact changes can not be compared at the moment - that's something that should probably be handled in a separate PR (might be non-trivial regarding changes in e.g. an entry's history).

@MathiasLui
Copy link

Ah, I thought your "clicking yes" meant the confirmation dialog. :)

Yes, it simply asked if I wanted to merge several times in a row with the options "Yes" and "No"

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

Successfully merging a pull request may close this issue.

8 participants