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

feat: user logging #13

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

feat: user logging #13

wants to merge 7 commits into from

Conversation

PeterBaker0
Copy link
Collaborator

@PeterBaker0 PeterBaker0 commented Dec 3, 2024

This adds basic user logging for logins, change passwords and change roles.

This route includes pagination.

Also bumps prisma to new major version v6 - no breaking changes for us.

Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Signed-off-by: Peter Baker <peter.baker122@csiro.au>
@PeterBaker0 PeterBaker0 requested a review from arlowhite December 3, 2024 22:24
Copy link

github-actions bot commented Dec 3, 2024

❌ Code formatting check failed. Please run npm run format:write locally and commit the changes.

Signed-off-by: Peter Baker <peter.baker122@csiro.au>
Copy link
Contributor

@arlowhite arlowhite left a comment

Choose a reason for hiding this comment

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

Looks good.

Was a bit concerned about initialiseAdmins move, but I think it's fine in this case.

import { initialiseAdmins } from './initialise';

console.log('Initializing admins...');
initialiseAdmins();
Copy link
Contributor

Choose a reason for hiding this comment

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

are you sure you want this here? previously, it was called by /init or src\infra\lambda.ts

With it running automatically like this you could have multiple instances running the function simultaneously. Typically I would add a lock to prevent this, but in this case the risk is minimal and we're not running multiple instances anyway. Redundant updates are fine and duplicate user inserts won't happen due to email unique constraint.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just a thought, but could consider doing this management with database migrations.

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.

2 participants