nicole/pique
1
0
Fork 0
Code Pull requests Activity

Fixes from code overview session #5

Merged
nicole merged 3 commits from code-review-fixes into main 2024-06-09 01:17:55 +00:00
Conversation 0 Commits 3 Files changed 6 +14
2 changed files with 14 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files
Showing only changes of commit adb3b3717e - Show all commits

6
Makefile
View file

@ -1,4 +1,10 @@
check:
cargo test
cargo fmt --check
cargo clippy
cargo build --release
run: run:
SECURE_SESSIONS=false RUST_LOG=debug cargo run -- --reload-templates SECURE_SESSIONS=false RUST_LOG=debug cargo run -- --reload-templates

8
src/permissions.rs
View file

@ -42,6 +42,14 @@ pub mod q {
}; };
if row_count > 0 { if row_count > 0 {
erika marked this conversation as resolved
erika commented 2024-06-06 03:56:11 +00:00
Review
Copy link

Do we have a scenario where row_count can be greater than 1?

Do we have a scenario where `row_count` can be greater than 1?
nicole commented 2024-06-09 00:59:57 +00:00
Review
Copy link

I don't think so. There's no constraint in the database to make it so that users are only members of a project once, but I can't imagine why we'd do that on purpose. It could be a good place to put an error log if the row count is over 1, while continuing to work.

I don't think so. There's no constraint in the database to make it so that users are only members of a project once, but I can't imagine why we'd do that on purpose. It could be a good place to put an error log if the row count is over 1, while continuing to work.
nicole commented 2024-06-09 01:11:42 +00:00
Review
Copy link

I added an error log. Open to changing it, erring on the side of merging.

I added an error log. Open to changing it, erring on the side of merging.
if row_count > 1 {
tracing::error!(
row_count = row_count,
user_id = user_id,
project_id = project_id,
"unexpected row count: more than one project membership for this user"
)
}
Ok(Decision::Allowed) Ok(Decision::Allowed)
} else { } else {
Ok(Decision::Denied) Ok(Decision::Denied)