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 +45 -33
nicole commented 2024-06-06 02:14:50 +00:00
Owner
Copy link

A collection of small changes:

  • Switch from using a boolean for permission check results to using an enum (allowed or denied)
  • Remove some unused Makefile commands left over from a previous migration
  • Change some hardcoded paths to be relative paths
  • Remove unused embed_migrations (duplicated)
A collection of small changes: - Switch from using a boolean for permission check results to using an enum (allowed or denied) - Remove some unused Makefile commands left over from a previous migration - Change some hardcoded paths to be relative paths - Remove unused embed_migrations (duplicated)
nicole added 2 commits 2024-06-06 02:14:51 +00:00 
Removed extraneous embedded migrations, added explicit relative path to migrations folder, removed hardcoded folder in diesel.toml
erika reviewed 2024-06-06 03:56:24 +00:00
src/permissions.rs
@ -23,3 +29,2 @@
if permission == Permission::Admin {
let is_admin = pm::project_memberships
let row_count = match requested_permission {
erika commented 2024-06-06 03:55:15 +00:00
Collaborator
Copy link

I like the switch to match here.

I like the switch to match here.
erika marked this conversation as resolved
src/permissions.rs
@ -39,2 +42,3 @@
};
Ok(is_member > 0)
if row_count > 0 {
erika commented 2024-06-06 03:56:11 +00:00
Collaborator
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
Author
Owner
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
Author
Owner
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.
erika marked this conversation as resolved
nicole added 1 commit 2024-06-09 01:12:18 +00:00
nicole merged commit 6ea4409d68 into main 2024-06-09 01:17:55 +00:00
nicole deleted branch code-review-fixes 2024-06-09 01:17:55 +00:00
nicole referenced this pull request from a commit 2024-06-09 01:17:57 +00:00
Fixes from code overview session (#5)
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: nicole/pique#5
No description provided.
Delete branch "code-review-fixes"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?