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

[users] Add 'request account date' to users #6191

Merged
merged 5 commits into from
Jul 21, 2020
Merged

[users] Add 'request account date' to users #6191

merged 5 commits into from
Jul 21, 2020

Conversation

cmadjar
Copy link
Collaborator

@cmadjar cmadjar commented Mar 26, 2020

Brief summary of changes

This adds a new account_request_date field to the users table to register information about when a given user requested an account.

Testing instructions

  1. source the SQL patch attached to the PR
  2. test that requesting a new account (on the login page) inserts a new user with the date properly filled
  3. test that the 'Account Request Date' is properly displayed in the menu filter page of user accounts
  4. check that the 'Account Request Date' is properly displayed after the 'Active from' and 'Active to' fields in the edit user page of user accounts.

Link(s) to related issue(s)

  • I don't believe an issue was created for that but this came up with Open PREVENT-AD where they wanted to have information about when a user requested access

@cmadjar cmadjar added Feature PR or issue introducing/requiring at least one new feature Caveat for Existing Projects PR contains changes that might impact the code or accepted practices of current active projects SQL PR contains SQL modifications such as schema changes and new SQL scripts RaisinBread PR or issue introducing/requiring improvements to the Raidinbread dataset Needs RB Update Update the raisinbread database to reflect changes in this PR Needs Changelog Update Update the CHANGELOG file to reflect changes in this PR labels Mar 26, 2020
@cmadjar
Copy link
Collaborator Author

cmadjar commented Mar 26, 2020

@ridz1208 @PapillonMcGill could you review this PR? I'll update the RB dataset after approval that the database changes are fine with you :). Thanks!

@cmadjar cmadjar removed the Needs Changelog Update Update the CHANGELOG file to reflect changes in this PR label Mar 26, 2020
cmadjar
cmadjar commented Mar 26, 2020
View reviewed changes
SQL/New_patches/2020-03-26_add_account_request_date_to_users_table.sql

-- determine the account_request_date based on what is present in the history
-- table for when the user was inserted the first time in the history table
UPDATE users u, history h
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ridz1208 is the assumption that all past users requested an account and therefore the date could be derived from the history table correct?

This was the case for Open PREVENT-AD but it might not necessarily be the case for all....

@cmadjar cmadjar removed Needs RB Update Update the raisinbread database to reflect changes in this PR RaisinBread PR or issue introducing/requiring improvements to the Raidinbread dataset labels Mar 27, 2020
@cmadjar
Copy link
Collaborator Author

cmadjar commented Mar 27, 2020

In the end, there is no need for raisin bread changes in this issue since the only user is admin and it does not have any request date associated to it. Therefore, I removed the RB labels.

@maltheism maltheism added the Passed Manual Tests PR has undergone proper testing by at least one peer label Mar 30, 2020
maltheism
maltheism approved these changes Mar 30, 2020
View reviewed changes
Copy link
Member

@maltheism maltheism left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passed manually testing for me.

driusan
driusan reviewed Apr 1, 2020
View reviewed changes
CHANGELOG.md Outdated
@@ -16,6 +16,8 @@ changes in the following format: PR #1234***
- Very old instruments relying on QuickForm may have issues due to code changes (PR #4928)
- Unix user permissions have been updated which may affect access to files. New
documentation for file permissions has been added to the README.md file (PR #5323)
- Addition of a new `account_request_date` in `users` table that will be used when
requesting a new account and will be displayed in the User Accounts module (PR #6191)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is in the wrong section of the CHANGELOG. It's not going into the 23.0 release..

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oooooooooh, oupsie...

@driusan should be fixed in the last commit. Sorry about that...

@johnsaigle johnsaigle added Needs Changelog Update Update the CHANGELOG file to reflect changes in this PR Needs Rebase PR contains conflicts with the current target branch or was issued to the wrong branch labels Apr 8, 2020
@driusan
Copy link
Collaborator

driusan commented Jul 13, 2020

@cmadjar can you rebase this?

@cmadjar cmadjar removed the Needs Changelog Update Update the CHANGELOG file to reflect changes in this PR label Jul 13, 2020
@cmadjar
add SQL statements in the SQL schema and new patch
a740397 
modifies the requestaccount class

modified menu filter and user account page to include the new date field

update the CHANGELOG

phpcs

modified UserTest to add the new column
@cmadjar cmadjar force-pushed the add_request_account_date_to_users branch from 4ffcc32 to a740397 Compare July 13, 2020 13:45
@cmadjar cmadjar removed the Needs Rebase PR contains conflicts with the current target branch or was issued to the wrong branch label Jul 13, 2020
@cmadjar
Copy link
Collaborator Author

cmadjar commented Jul 13, 2020

Fascinating. I did not even remember sending that, lol
@driusan rebased now. Thanks for tagging me!

@driusan
Copy link
Collaborator

driusan commented Jul 13, 2020

@cmadjar the changelog conflict wasn't resolved correctly.. it should be in the 24.0 part of the changelog, not the 23.

@cmadjar cmadjar changed the base branch from master to main July 21, 2020 15:10
driusan
driusan requested changes Jul 21, 2020
View reviewed changes
CHANGELOG.md Outdated
@@ -43,6 +45,7 @@ Quality Control, and Behavioural Quality Control. (PR #6041)
database (PR #5260)
- New documentation for file permissions has been added to the README.md file. (PR #5323)
- Dashboard study progression section performance improvement (PR #5887)
documentation for file permissions has been added to the README.md file (PR #5323)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this line is supposed to be here.. it's a duplicate of 2 lines above it (but the formating is also messed u..)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch! I removed the line. Sorry about that...

@driusan driusan merged commit 4fdcf0f into aces:main Jul 21, 2020
@ridz1208 ridz1208 added this to the 24.0.0 milestone Jul 27, 2020
@spell00 spell00 mentioned this pull request Aug 11, 2020
Closed
spell00 pushed a commit to spell00/Loris that referenced this pull request Aug 13, 2020
@cmadjar @spell00
[users] Add 'request account date' to users (aces#6191)
cce2c32 
This adds a new account_request_date field to the users table to register information about when a given user requested an account.
@laemtl laemtl mentioned this pull request Sep 23, 2020
Closed
AlexandraLivadas pushed a commit to AlexandraLivadas/Loris that referenced this pull request Jun 15, 2021
@cmadjar @AlexandraLivadas
[users] Add 'request account date' to users (aces#6191)
f514f4e 
This adds a new account_request_date field to the users table to register information about when a given user requested an account.
AlexandraLivadas pushed a commit to AlexandraLivadas/Loris that referenced this pull request Jun 29, 2021
@cmadjar @AlexandraLivadas
[users] Add 'request account date' to users (aces#6191)
73f90d4 
This adds a new account_request_date field to the users table to register information about when a given user requested an account.
@cmadjar cmadjar deleted the add_request_account_date_to_users branch September 24, 2021 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@driusan driusan driusan requested changes

@maltheism maltheism maltheism approved these changes

@PapillonMcGill PapillonMcGill Awaiting requested review from PapillonMcGill

@ridz1208 ridz1208 Awaiting requested review from ridz1208

Assignees

@cmadjar cmadjar

Labels
Caveat for Existing Projects PR contains changes that might impact the code or accepted practices of current active projects Feature PR or issue introducing/requiring at least one new feature Passed Manual Tests PR has undergone proper testing by at least one peer SQL PR contains SQL modifications such as schema changes and new SQL scripts
Projects
None yet
Milestone
24.0.0
Development

Successfully merging this pull request may close these issues.
5 participants
@cmadjar @driusan @maltheism @johnsaigle @ridz1208