[47] Roles endpoints #50

Mateusz-Dobrzynski · 2025-02-28T17:37:15Z

Resolves #47. Builds up on #46 and #49.

…les-endpoints

jakubmanczak

Partial; will return to this in the evening.

src/tournament/roles.rs

src/routes/roles_routes.rs

jakubmanczak

Few things I caught, incl. a major critical auth logic bug.
Would also be nice to get rid of the unused import warnings - they've multiplied to be quite numerous by now.

jakubmanczak · 2025-10-15T18:52:04Z

src/routes/roles_routes.rs

+    match tournament_user.roles.is_empty() {
+        true => (),
+        false => return Err(OmniError::UnauthorizedError),
+    }


I don't think that was what you meant to do here - this would formerly reject users with roles and allow through users without them.

Suggested change

match tournament_user.roles.is_empty() {

true => (),

false => return Err(OmniError::UnauthorizedError),

}

if tournament_user.roles.is_empty() {

return Err(OmniError::UnauthorizedError);

}

jakubmanczak · 2025-10-27T21:14:34Z

src/omni_error.rs

    BadRequestError,
    #[error("{INSUFFICIENT_PERMISSIONS_MESSAGE}")]
    InsufficientPermissionsError,
+    #[error("ROLES_PARSING_MESSAGE")]


Suggested change

#[error("ROLES_PARSING_MESSAGE")]

#[error("{ROLES_PARSING_MESSAGE}")]

jakubmanczak · 2025-10-27T21:14:44Z

src/omni_error.rs

    InsufficientPermissionsError,
+    #[error("ROLES_PARSING_MESSAGE")]
+    RolesParsingError,
    #[error("REFERRING_TO_A_NONEXISTENT_RESOURCE")]


Suggested change

#[error("REFERRING_TO_A_NONEXISTENT_RESOURCE")]

#[error("{REFERRING_TO_A_NONEXISTENT_RESOURCE}")]

jakubmanczak · 2025-10-27T21:29:02Z

src/routes/roles_routes.rs

+                "Error deleting roles of user {} within tournament {}: {e}",
+                user_id, tournament_id
+            );
+            Err(e)?


Suggested change

Err(e)?

Err(e)

jakubmanczak · 2025-10-27T21:29:10Z

src/routes/roles_routes.rs

+                "Error getting roles of user {} within tournament {}: {e}",
+                user_id, tournament_id
+            );
+            Err(e)?


Suggested change

Err(e)?

Err(e)

jakubmanczak · 2025-10-27T21:29:18Z

src/routes/roles_routes.rs

+                "Error creating roles for user {} within tournament {}: {e}",
+                user_id, tournament_id
+            );
+            Err(e)?


Suggested change

Err(e)?

Err(e)

jakubmanczak · 2025-10-27T21:30:00Z

src/tournament/roles.rs

+    ) -> Result<Vec<Role>, OmniError> {
+        let _ = tournament_id;
+        let roles_as_strings = Role::roles_vec_to_string_vec(&roles);
+        roles_as_strings.iter().for_each(|r| info!(r));


Why are we logging every time? This will be straight spam.

jakubmanczak · 2025-10-27T21:38:47Z

src/tournament/roles.rs

+        let mut string_vec: Vec<String> = vec![];
+        roles
+            .iter()
+            .for_each(|role| string_vec.push(role.to_string()));
+        return string_vec;


Suggested change

let mut string_vec: Vec<String> = vec![];

roles

.iter()

.for_each(|role| string_vec.push(role.to_string()));

return string_vec;

let string_vec: Vec<String> = roles.iter().map(|r| r.to_string()).collect();

return string_vec;

If implementing the trait below, obviously remove the method.

jakubmanczak · 2025-10-27T22:34:16Z

src/tournament/roles.rs

+        pool: &Pool<Postgres>,
+    ) -> Result<Vec<Role>, OmniError> {
+        let _ = tournament_id;
+        let roles_as_strings = Role::roles_vec_to_string_vec(&roles);


Would be nicer to implement a Vec<Role> to Vec<String> converter via an extending trait, rather than make a method on Role that takes in a Vec as arg...
See suggestion and relevant impl below.

Suggested change

let roles_as_strings = Role::roles_vec_to_string_vec(&roles);

let roles_as_strings = roles.to_string_vec();

jakubmanczak · 2025-10-27T22:34:32Z

src/tournament/roles.rs

+            _ => Err(OmniError::RolesParsingError),
+        }
+    }
+}


Would be nicer to implement a Vec to Vec converter via an extending trait, rather than make a method on Role that takes in a Vec as arg...

Suggested change

}

}

trait RoleVecExt {

fn to_string_vec(&self) -> Vec<String>;

}

impl RoleVecExt for Vec<Role> {

fn to_string_vec(&self) -> Vec<String> {

self.iter().map(|role| role.to_string()).collect()

}

}

jakubmanczak · 2025-11-03T18:22:51Z

It would be nice not to implement Display and FromStr for unit enums by hand. The strum crate does this for us, but this can definitely be done in a future pull request as it will require integration of strum errors into OmniError.

jakubmanczak · 2025-11-03T18:26:21Z

Also; does your autoformatting work? When testing this PR locally a lot of changes are made on save. That's not supposed to happen.

jakubmanczak · 2025-11-03T18:29:09Z

Merging master into this branch is also in order.

Mateusz-Dobrzynski added 5 commits February 26, 2025 14:57

[48] implement user endpoints

a2afede

[48] code cleanup

e657c6d

[48] fix user patch endpoint

7f92507

[47] implement roles endpoints

d72ce66

[47] code cleanup

d186f6d

Mateusz-Dobrzynski requested a review from jakubmanczak February 28, 2025 17:37

Mateusz-Dobrzynski self-assigned this Feb 28, 2025

Mateusz-Dobrzynski linked an issue Feb 28, 2025 that may be closed by this pull request

Roles endpoints #47

Open

Mateusz-Dobrzynski added 3 commits February 28, 2025 18:46

[47] docs cleanup

7d0b8d1

Merge branch 'master' of https://github.com/debatecore/tau into 47-ro…

ba0ddee

…les-endpoints

[47] post-merge fixes

e7f1dbf

Mateusz-Dobrzynski mentioned this pull request Mar 14, 2025

[53] Judges' affiliations #58

Open

Mateusz-Dobrzynski added 5 commits August 13, 2025 15:33

Merge branch 'master' of https://github.com/debatecore/tau into 47-ro…

005dd66

…les-endpoints

[47] prepare sqlx queries

037d22d

[47] prepare sqlx queries (reprise)

42eb4d9

Merge branch 'master' of https://github.com/debatecore/tau into 47-ro…

74b113f

…les-endpoints

[47] post-merge fixes

93be087

jakubmanczak reviewed Sep 15, 2025

View reviewed changes

src/tournament/roles.rs Outdated Show resolved Hide resolved

src/tournament/roles.rs Outdated Show resolved Hide resolved

src/routes/roles_routes.rs Outdated Show resolved Hide resolved

[47] apply code review suggestions

b3d9703

Mateusz-Dobrzynski requested a review from jakubmanczak September 16, 2025 11:44

jakubmanczak requested changes Oct 27, 2025

View reviewed changes

	#[error("ROLES_PARSING_MESSAGE")]
	#[error("{ROLES_PARSING_MESSAGE}")]

	#[error("REFERRING_TO_A_NONEXISTENT_RESOURCE")]
	#[error("{REFERRING_TO_A_NONEXISTENT_RESOURCE}")]

	let roles_as_strings = Role::roles_vec_to_string_vec(&roles);
	let roles_as_strings = roles.to_string_vec();

-}
+}
+trait RoleVecExt {
+    fn to_string_vec(&self) -> Vec<String>;
+}
+impl RoleVecExt for Vec<Role> {
+    fn to_string_vec(&self) -> Vec<String> {
+        self.iter().map(|role| role.to_string()).collect()
+    }
+}

[47] Roles endpoints #50

Are you sure you want to change the base?

[47] Roles endpoints #50

Uh oh!

Conversation

Mateusz-Dobrzynski commented Feb 28, 2025

Uh oh!

jakubmanczak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jakubmanczak left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakubmanczak Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakubmanczak Oct 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jakubmanczak commented Nov 3, 2025

Uh oh!

jakubmanczak commented Nov 3, 2025

Uh oh!

jakubmanczak commented Nov 3, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jakubmanczak left a comment •

edited

Loading

jakubmanczak Oct 27, 2025 •

edited

Loading

jakubmanczak Oct 27, 2025 •

edited

Loading