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

Jump to bottom

KeeShare - improve share observer code #3029

Merged
merged 1 commit into from
Jun 20, 2019
Merged

KeeShare - improve share observer code #3029

merged 1 commit into from
Jun 20, 2019

Conversation

ckieschnick
Copy link
Contributor

Introduce a path resolver to allow to specify shares relative to the database (#2799).

Type of change

  • ✅ Bug fix (non-breaking change which fixes an issue)
  • ✅ Refactor (significant modification to existing code)

Description and Context

ShareObserver now tracks the location of shares using their path translated relative to the share defining database. This means absolute paths (which are still default using the file dialog) will work, but manually defined share location using relatives path should work now to.

Since ShareObserver exploded which made it hard do maintain, I splitted the functionality into ShareImport and ShareExport, both encapsulating their respective code parts which are just called from the ShareObserver.
To prevent "retranslation" the new classes use ShareObserver::tr to translate the messages.

Since the refactoring took place in a second commit, feel free to just use the patch of the first commit.

Testing strategy

Manually tested.

Checklist:

  • ✅ I have read the CONTRIBUTING document. [REQUIRED]
  • ✅ My code follows the code style of this project. [REQUIRED]
  • ✅ All new and existing tests passed. [REQUIRED]

@ckieschnick ckieschnick mentioned this pull request Apr 18, 2019
Closed
@droidmonkey
Copy link
Member

droidmonkey commented Apr 18, 2019 •
edited
Loading

This is fantastic, can you rebase on develop? I want to target refactors to 2.5.0

@droidmonkey droidmonkey added feature: KeeShare pr: refactoring Pull request that refactors code labels Apr 18, 2019
@droidmonkey droidmonkey added this to the v2.5.0 milestone Apr 18, 2019
@ckieschnick
Copy link
Contributor Author

ckieschnick commented Apr 18, 2019 •
edited
Loading

In this case I would split the request? One with the minimal patch for release 2.4.2, the other for 2.5 based on develop?
This way, I would suggest to remove the ShareObserver::tr workaround and introduce normal translation using tr in ShareImport and ShareExport. Are you okay with this?

@ckieschnick ckieschnick force-pushed the hotfix/2799-keeshare-with-relative-paths branch 3 times, most recently from 2bc843f to 2ed54db Compare April 18, 2019 12:36
@droidmonkey
Copy link
Member

droidmonkey commented Apr 18, 2019 •
edited
Loading

Yes we should always use normal tr, you can always call QObject::tr(...) as a last resort. Please do split the PR. Just keep this one as is (with the tr fix)

@ckieschnick ckieschnick mentioned this pull request Apr 18, 2019
Merged
@ckieschnick ckieschnick force-pushed the hotfix/2799-keeshare-with-relative-paths branch from 2ed54db to 2f95153 Compare April 18, 2019 13:03
@ckieschnick ckieschnick changed the base branch from release/2.4.2 to develop April 18, 2019 13:04
@ckieschnick ckieschnick changed the title Hotfix/2799 keeshare with relative paths Hotfix/2799 keeshare improved share observer Apr 18, 2019
@ckieschnick
Copy link
Contributor Author

I'll do the tr probably next week. Just changed the title to reflect the real changes. The patch for 2.4.2 is in #3031.

@ckieschnick ckieschnick force-pushed the hotfix/2799-keeshare-with-relative-paths branch 2 times, most recently from 1fe5731 to e851128 Compare April 26, 2019 11:01
@droidmonkey
Copy link
Member

@ckieschnick is this ready for review? Will need to be rebased.

@droidmonkey droidmonkey changed the title Hotfix/2799 keeshare improved share observer KeeShare - improve share observer code Jun 20, 2019
@droidmonkey droidmonkey self-requested a review June 20, 2019 01:54
@droidmonkey droidmonkey force-pushed the hotfix/2799-keeshare-with-relative-paths branch 2 times, most recently from 3e690ad to c2076cd Compare June 20, 2019 02:12
@droidmonkey
Extract ShareImport and ShareExport
791cb87 
Moved import/export functionality out of ShareObserver into tooling classes ShareImport and ShareExport
@droidmonkey droidmonkey force-pushed the hotfix/2799-keeshare-with-relative-paths branch from c2076cd to 791cb87 Compare June 20, 2019 02:19
@droidmonkey droidmonkey merged commit 07da5de into keepassxreboot:develop Jun 20, 2019
@ckieschnick ckieschnick deleted the hotfix/2799-keeshare-with-relative-paths branch August 12, 2019 16:08
phoerious added a commit that referenced this pull request Oct 26, 2019
@phoerious
Release 2.5.0
1ab8a9f 
Added

- Add 'Paper Backup' aka 'Export to HTML file' to the 'Database' menu [[#3277](#3277)]
- Add statistics panel with information about the database (number of entries, number of unique passwords, etc.) to the Database Settings dialog [[#2034](#2034)]
- Add offline user manual accessible via the 'Help' menu [[#3274](#3274)]
- Add support for importing 1Password OpVault files [[#2292](#2292)]
- Implement Freedesktop.org secret storage DBus protocol so that KeePassXC can be used as a vault service by libsecret [[#2726](#2726)]
- Add support for OnlyKey as an alternative to YubiKeys (requires yubikey-personalization >= 1.20.0) [[#3352](#3352)]
- Add group sorting feature [[#3282](#3282)]
- Add feature to download favicons for all entries at once [[#3169](#3169)]
- Add word case option to passphrase generator [[#3172](#3172)]
- Add support for RFC6238-compliant TOTP hashes [[#2972](#2972)]
- Add UNIX man page for main program [[#3665](#3665)]
- Add 'Monospaced font' option to the notes field [[#3321](#3321)]
- Add support for key files in auto open [[#3504](#3504)]
- Add search field for filtering entries in Auto-Type dialog [[#2955](#2955)]
- Complete usernames based on known usernames from other entries [[#3300](#3300)]
- Parse hyperlinks in the notes field of the entry preview pane [[#3596](#3596)]
- Allow abbreviation of field names in entry search [[#3440](#3440)]
- Allow setting group icons recursively [[#3273](#3273)]
- Add copy context menu for username and password in Auto-Type dialog [[#3038](#3038)]
- Drop to background after copying a password to the clipboard [[#3253](#3253)]
- Add 'Lock databases' entry to tray icon menu [[#2896](#2896)]
- Add option to minimize window after unlocking [[#3439](#3439)]
- Add option to minimize window after opening a URL [[#3302](#3302)]
- Request accessibility permissions for Auto-Type on macOS [[#3624](#3624)]
- Browser: Add initial support for multiple URLs [[#3558](#3558)]
- Browser: Add entry-specific browser integration settings [[#3444](#3444)]
- CLI: Add offline HIBP checker (requires a downloaded HIBP dump) [[#2707](#2707)]
- CLI: Add 'flatten' option to the 'ls' command [[#3276](#3276)]
- CLI: Add password generation options to `Add` and `Edit` commands [[#3275](#3275)]
- CLI: Add XML import [[#3572](#3572)]
- CLI: Add CSV export to the 'export' command [[#3278](#3278)]
- CLI: Add `-y --yubikey` option for YubiKey [[#3416](#3416)]
- CLI: Add `--dry-run` option for merging databases [[#3254](#3254)]
- CLI: Add group commands (mv, mkdir and rmdir) [[#3313](#3313)].
- CLI: Add interactive shell mode command `open` [[#3224](#3224)]

Changed

- Redesign database unlock dialog [ [#3287](#3287)]
- Rework the entry preview panel [ [#3306](#3306)]
- Move notes to General tab on Group Preview Panel [[#3336](#3336)]
- Enable entry actions when editing an entry and cleanup entry context menu  [[#3641](#3641)]
- Improve detection of external database changes  [[#2389](#2389)]
- Warn if user is trying to use a KDBX file as a key file [[#3625](#3625)]
- Add option to disable KeePassHTTP settings migrations prompt [[#3349](#3349), [#3344](#3344)]
- Re-enabled Wayland support (no Auto-Type yet) [[#3520](#3520), [#3341](#3341)]
- Add icon to 'Toggle Window' action in tray icon menu [[3244](#3244)]
- Merge custom data between databases only when necessary [[#3475](#3475)]
- Improve various file-handling related issues when picking files using the system's file dialog [[#3473](#3473)]
- Add 'New Entry' context menu when no entries are selected [[#3671](#3671)]
- Reduce default Argon2 settings from 128 MiB and one thread per CPU core to 64 MiB and two threads to account for lower-spec mobile hardware [ [#3672](#3672)]
- Browser: Remove unused 'Remember' checkbox for HTTP Basic Auth [[#3371](#3371)]
- Browser: Show database name when pairing with a new browser [[#3638](#3638)]
- Browser: Show URL in allow access dialog [[#3639](#3639)]
- CLI: The password length option `-l` for the CLI commands `Add` and `Edit` is now `-L` [[#3275](#3275)]
- CLI: The `-u` shorthand for the `--upper` password generation option has been renamed to `-U` [[#3275](#3275)]
- CLI: Rename command `extract` to `export`. [[#3277](#3277)]

Fixed

- Improve accessibility for assistive technologies [[#3409](#3409)]
- Correctly unlock all databases if `--pw-stdin` is provided [[#2916](#2916)]
- Fix password generator issues with special characters [[#3303](#3303)]
- Fix KeePassXC interrupting shutdown procedure [[#3666](#3666)]
- Fix password visibility toggle button state on unlock dialog [[#3312](#3312)]
- Fix potential data loss if database is reloaded while user is editing an entry [[#3656](#3656)]
- Fix hard-coded background color in search help popup [[#3001](#3001)]
- Fix font choice for password preview [[#3425](#3425)]
- Fix handling of read-only files when autosave is enabled [[#3408](#3408)]
- Handle symlinks correctly when atomic saves are disabled [[#3463](#3463)]
- Enable HighDPI icon scaling on Linux [[#3332](#3332)]
- Make Auto-Type on macOS more robust and remove old Carbon API calls [[#3634](#3634), [[#3347](#3347))]
- Hide Share tab if KeePassXC is compiled without KeeShare support and other minor KeeShare improvements [[#3654](#3654), [[#3291](#3291), [#3029](#3029), [#3031](#3031), [#3236](#3236)]
- Correctly bring window to the front when clicking tray icon on macOS [[#3576](#3576)]
- Correct application shortcut created by MSI Installer on Windows [[#3296](#3296)]
- Fix crash when removing custom data [[#3508](#3508)]
- Fix placeholder resolution in URLs [[#3281](#3281)]
- Fix various inconsistencies and platform-dependent compilation bugs [[#3664](#3664), [#3662](#3662), [#3660](#3660), [#3655](#3655), [#3649](#3649), [#3417](#3417), [#3357](#3357), [#3319](#3319), [#3318](#3318), [#3304](#3304)]
- Browser: Fix potential leaking of entries through the browser integration API if multiple databases are opened [[#3480](#3480)]
- Browser: Fix password entropy calculation [[#3107](#3107)]
- Browser: Fix Windows registry settings for portable installation [[#3603](#3603)]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@droidmonkey droidmonkey droidmonkey approved these changes

Assignees
No one assigned
Labels
feature: KeeShare pr: refactoring Pull request that refactors code
Projects
None yet
Milestone
v2.5.0
Development

Successfully merging this pull request may close these issues.
2 participants
@ckieschnick @droidmonkey