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

feature/passwdExpDaysDiff #7029

Closed

Conversation

shemeshg
Copy link
Contributor

@shemeshg shemeshg commented Oct 8, 2021

Added Days to expiration date of PasswordEntry and GroupEntry
NOTE: # ( Explain large or complex code modifications. )
NOTE: # ( If it fixes an open issue, please add "Fixes #XXX" )
Fixes #4896

Screenshots

Screen Shot 2021-10-08 at 19 30 53
Screen Shot 2021-10-08 at 19 30 40

Testing strategy

  1. On the Entry
    1.1 Change "expireDateInDays" see it affect "expireDatePicker" correctly.
    1.2 Change "expireDatePicker" see it affect "expireDateInDays" correctly.
    1.3 "expireDateInDays" min value is 0 (no negative)
    1.4 Use preset button effect "expireDatePicker" and "expireDateInDays" correctly.
  2. Repeat step 1.1, 1.2 1.3 for Group Entry

N/A

Type of change

  • ✅ Bug fix (non-breaking change that fixes an issue)
  • ✅ New feature (change that adds functionality)

@shemeshg
Copy link
Contributor Author

shemeshg commented Oct 8, 2021

It is a bit simplified,
having it in weeks and months would require more effort of coordinating the changes of three controls

  • Dropdown.
  • Spinner
  • and Date
    and maybe even storing the value of the additional dropbox, and verbose UI

Tell me if it is sufficient or Over simplified

@droidmonkey
Copy link
Member

I'm confused, I'd this a static value or meant to be a user editable field? If static, this should just appear on the entry preview panel.

@shemeshg
Copy link
Contributor Author

shemeshg commented Oct 8, 2021

I'm confused, I'd this a static value or meant to be a user editable field? If static, this should just appear on the entry preview panel.

It is "user editable field",

  • the days field is bound to the DateTime Drop down
  • and vice versa

You can set expiration date with the Days QSpinner or the QDateTimeEdit

See "Testing strategy" above

@codecov-commenter
Copy link

codecov-commenter commented Oct 9, 2021

Codecov Report

Merging #7029 (26b16d0) into develop (6c18b10) will increase coverage by 0.00%.
The diff coverage is 66.67%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #7029   +/-   ##
========================================
  Coverage    63.68%   63.68%           
========================================
  Files          330      330           
  Lines        41601    41619   +18     
========================================
+ Hits         26491    26503   +12     
- Misses       15110    15116    +6     
Impacted Files Coverage Δ
src/gui/entry/EditEntryWidget.cpp 67.58% <55.56%> (-0.09%) ⬇️
src/gui/group/EditGroupWidget.cpp 78.54% <77.78%> (-0.03%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6c18b10...26b16d0. Read the comment docs.

@shemeshg
Copy link
Contributor Author

shemeshg commented Oct 12, 2021

Hi

I've just seen: #6456

Though it is different because it is for Password Change only, yet:
Maybe current PR could be integrated into #6456
And these reactive QSpinner of "Days" integrated to the new rubric is designed in #6456
(and the EditGroup dialog [that is not part of the #6456 as I saw] )

Or even better maybe consider this PR to be rewritten as part of #6456 by @xvallspl
Since

  1. these are really small changes of few lines.
  2. merging of these UI files would yield a conflict with no sense to resolve (except manually adding the QSpinner).

Tags: #2010 #6456

@droidmonkey
Copy link
Member

Closing this in favor of the above solution.

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.

Password expiration presets should be in DAYS
3 participants