-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat(commands): add creation timestamp and user name inside commands #2252
feat(commands): add creation timestamp and user name inside commands #2252
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I talked with Alexander and put the conclusion inside the JIRA ticket but basically wehave to do the conversion in the back from user_id to user name. Also ,we have to do it cleverly meaning if we retrieve 500 commands don't do 500 requests to have the user name but rather one for each individual user
alembic/versions/a8e9ee2cd4be_add_user_and_updated_date_info_to_.py
Outdated
Show resolved
Hide resolved
alembic/versions/a8e9ee2cd4be_add_user_and_updated_date_info_to_.py
Outdated
Show resolved
Hide resolved
0b25e3e
to
8c840ba
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code seems okay but you'll have to add integration tests.
- Tests with 2 users altering the same study, you retrieve their names.
- Tests that one user appending two commands only call once the DB for user name
- Tests that it works with bots (add the test inside test_integration_token_end_to_end)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor reviews, it seems almost done :)
…l, add migration file
…API, update tests
ee86077
to
c5fd412
Compare
…copy variant methods, update comments
Resolve ANT-1705