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
30473344af Fix slop in DB code. 
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
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
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?