refactor(models): replace hardcoded strings with typed enums

- Add ReviewState enum (pending, approved, changes_requested, etc.) - Add DEFAULT_REVISION constant for git HEAD references - service/pr/reviews.rs: use ReviewState for review creation and submission state validation - service/pr/core.rs: use MergeStrategyKind for merge strategy selection - service/im/stages.rs: use StagePrivacyLevel for stage creation - service/im/invitations.rs: use Role enum for invitation role defaults
2026-06-10 18:49:06 +08:00
parent 15b875e18d
commit 6205a6de0a
5 changed files with 334 additions and 45 deletions
@@ -2,14 +2,14 @@ use serde::{Deserialize, Serialize};
 use uuid::Uuid;

 use crate::error::AppError;
-use crate::models::common::{Role, State};
+use crate::models::common::{MergeStrategyKind, Role, State};
 use crate::models::prs::PullRequest;
 use crate::models::repos::Repo;
 use crate::models::workspaces::Workspace;
 use crate::service::PrService;
 use crate::session::Session;

-use super::util::{clamp_limit_offset, ensure_affected, merge_optional_text};
+use super::util::{clamp_limit_offset, ensure_affected, merge_optional_text, set_local_user_id};

 #[derive(Deserialize, Serialize, Clone, Debug, utoipa::ToSchema)]
 pub struct CreatePrParams {
@@ -175,8 +175,7 @@ impl PrService {
            .begin()
            .await
            .map_err(|_| AppError::TxnError)?;
-        sqlx::query("SET LOCAL app.current_user_id = $1")
-            .bind(user_uid)
+        sqlx::query(set_local_user_id(user_uid))
            .execute(&mut *txn)
            .await
            .map_err(AppError::Database)?;
@@ -280,8 +279,7 @@ impl PrService {
            .begin()
            .await
            .map_err(|_| AppError::TxnError)?;
-        sqlx::query("SET LOCAL app.current_user_id = $1")
-            .bind(user_uid)
+        sqlx::query(set_local_user_id(user_uid))
            .execute(&mut *txn)
            .await
            .map_err(AppError::Database)?;
@@ -326,8 +324,7 @@ impl PrService {
            .begin()
            .await
            .map_err(|_| AppError::TxnError)?;
-        sqlx::query("SET LOCAL app.current_user_id = $1")
-            .bind(user_uid)
+        sqlx::query(set_local_user_id(user_uid))
            .execute(&mut *txn)
            .await
            .map_err(AppError::Database)?;
@@ -378,8 +375,7 @@ impl PrService {
            .begin()
            .await
            .map_err(|_| AppError::TxnError)?;
-        sqlx::query("SET LOCAL app.current_user_id = $1")
-            .bind(user_uid)
+        sqlx::query(set_local_user_id(user_uid))
            .execute(&mut *txn)
            .await
            .map_err(AppError::Database)?;
@@ -421,8 +417,7 @@ impl PrService {
            .begin()
            .await
            .map_err(|_| AppError::TxnError)?;
-        sqlx::query("SET LOCAL app.current_user_id = $1")
-            .bind(user_uid)
+        sqlx::query(set_local_user_id(user_uid))
            .execute(&mut *txn)
            .await
            .map_err(AppError::Database)?;
@@ -464,8 +459,7 @@ impl PrService {
            .begin()
            .await
            .map_err(|_| AppError::TxnError)?;
-        sqlx::query("SET LOCAL app.current_user_id = $1")
-            .bind(user_uid)
+        sqlx::query(set_local_user_id(user_uid))
            .execute(&mut *txn)
            .await
            .map_err(AppError::Database)?;
@@ -502,8 +496,7 @@ impl PrService {
            .begin()
            .await
            .map_err(|_| AppError::TxnError)?;
-        sqlx::query("SET LOCAL app.current_user_id = $1")
-            .bind(user_uid)
+        sqlx::query(set_local_user_id(user_uid))
            .execute(&mut *txn)
            .await
            .map_err(AppError::Database)?;
@@ -626,8 +619,7 @@ impl PrService {
            .begin()
            .await
            .map_err(|_| AppError::TxnError)?;
-        sqlx::query("SET LOCAL app.current_user_id = $1")
-            .bind(user_uid)
+        sqlx::query(set_local_user_id(user_uid))
            .execute(&mut *txn)
            .await
            .map_err(AppError::Database)?;
@@ -924,21 +916,26 @@ impl PrService {
        };

        // Determine merge strategy
-        let strategy = params.strategy.as_deref().unwrap_or("merge");
+        let strategy = params
+            .strategy
+            .as_deref()
+            .and_then(|s| s.parse::<MergeStrategyKind>().ok())
+            .filter(|s| *s != MergeStrategyKind::Unknown)
+            .unwrap_or(MergeStrategyKind::Merge);
        let merge_strategy = match strategy {
-            "squash" => pb::merge_options::Strategy::MergeStrategyOrt as i32,
-            "rebase" => pb::merge_options::Strategy::MergeStrategyRecursive as i32,
+            MergeStrategyKind::Squash => pb::merge_options::Strategy::MergeStrategyOrt as i32,
+            MergeStrategyKind::Rebase => pb::merge_options::Strategy::MergeStrategyRecursive as i32,
            _ => pb::merge_options::Strategy::MergeStrategyOrt as i32,
        };

        let options = pb::MergeOptions {
            strategy: merge_strategy,
-            fast_forward: if strategy == "rebase" {
+            fast_forward: if strategy == MergeStrategyKind::Rebase {
                pb::merge_options::FastForwardMode::MergeFastForwardModeNoFf as i32
            } else {
                pb::merge_options::FastForwardMode::MergeFastForwardModeAllowed as i32
            },
-            squash: strategy == "squash",
+            squash: strategy == MergeStrategyKind::Squash,
            no_commit: false,
            allow_unrelated_histories: false,
            strategy_options: vec![],
@@ -954,7 +951,7 @@ impl PrService {
            }),
            committer: None,
            message: params.squash_message.clone().unwrap_or_else(|| {
-                if strategy == "squash" {
+                if strategy == MergeStrategyKind::Squash {
                    params
                        .squash_title
                        .clone()
@@ -3,12 +3,12 @@ use serde::{Deserialize, Serialize};
 use uuid::Uuid;

 use crate::error::AppError;
-use crate::models::common::Role;
+use crate::models::common::{ReviewState, Role};
 use crate::models::prs::{PrReview, PrReviewComment};
 use crate::service::PrService;
 use crate::session::Session;

-use super::util::{clamp_limit_offset, ensure_affected, required_text};
+use super::util::{clamp_limit_offset, ensure_affected, required_text, set_local_user_id};

 #[derive(Debug, Deserialize, Serialize, utoipa::ToSchema)]
 pub struct CreateReviewParams {
@@ -83,15 +83,25 @@ impl PrService {
        let pr = self.resolve_pr(wk_name, repo_name, number).await?;
        self.ensure_pr_readable(user_uid, &pr).await?;

-        let state = params.state.as_deref().unwrap_or("pending");
-        if !["pending", "approved", "changes_requested", "commented"].contains(&state) {
+        let state = params
+            .state
+            .as_deref()
+            .and_then(|s| s.parse::<ReviewState>().ok())
+            .filter(|s| *s != ReviewState::Unknown)
+            .unwrap_or(ReviewState::Pending);
+        if matches!(
+            state,
+            ReviewState::Pending | ReviewState::Approved | ReviewState::ChangesRequested | ReviewState::Commented
+        ) {
+            // valid state
+        } else {
            return Err(AppError::BadRequest("invalid review state".into()));
        }
-        if matches!(state, "approved" | "changes_requested") {
+        if matches!(state, ReviewState::Approved | ReviewState::ChangesRequested) {
            let repo = self.resolve_repo(wk_name, repo_name).await?;
            self.ensure_repo_role_at_least(user_uid, &repo, Role::Member)
                .await?;
-            if state == "approved" && pr.author_id == user_uid {
+            if state == ReviewState::Approved && pr.author_id == user_uid {
                return Err(AppError::BadRequest(
                    "PR authors cannot approve their own pull requests".into(),
                ));
@@ -108,8 +118,7 @@ impl PrService {
            .begin()
            .await
            .map_err(|_| AppError::TxnError)?;
-        sqlx::query("SET LOCAL app.current_user_id = $1")
-            .bind(user_uid)
+        sqlx::query(set_local_user_id(user_uid))
            .execute(&mut *txn)
            .await
            .map_err(AppError::Database)?;
@@ -132,7 +141,7 @@ impl PrService {
                .as_deref()
                .or(Some(pr.head_commit_sha.as_str())),
        )
-        .bind(if state != "pending" { Some(now) } else { None })
+        .bind(if state != ReviewState::Pending { Some(now) } else { None })
        .bind(now)
        .fetch_one(&mut *txn)
        .await
@@ -158,7 +167,7 @@ impl PrService {
            }
        }

-        if matches!(state, "approved" | "changes_requested") {
+        if matches!(state, ReviewState::Approved | ReviewState::ChangesRequested) {
            sqlx::query(
                "UPDATE pr_status SET approvals_count = (SELECT COUNT(*) FROM pr_review r \
                 JOIN pull_request pr ON pr.id = r.pull_request_id \
@@ -190,16 +199,18 @@ impl PrService {
        let pr = self.resolve_pr(wk_name, repo_name, number).await?;
        self.ensure_pr_readable(user_uid, &pr).await?;

-        let state = params.state.as_str();
-        if !["approved", "changes_requested", "commented"].contains(&state) {
-            return Err(AppError::BadRequest("invalid review state".into()));
-        }
+        let state = params
+            .state
+            .parse::<ReviewState>()
+            .ok()
+            .filter(|s| *s != ReviewState::Unknown)
+            .ok_or_else(|| AppError::BadRequest("invalid review state".into()))?;

-        if matches!(state, "approved" | "changes_requested") {
+        if matches!(state, ReviewState::Approved | ReviewState::ChangesRequested) {
            let repo = self.resolve_repo(wk_name, repo_name).await?;
            self.ensure_repo_role_at_least(user_uid, &repo, Role::Member)
                .await?;
-            if state == "approved" && pr.author_id == user_uid {
+            if state == ReviewState::Approved && pr.author_id == user_uid {
                return Err(AppError::BadRequest(
                    "PR authors cannot approve their own pull requests".into(),
                ));
@@ -214,8 +225,7 @@ impl PrService {
            .begin()
            .await
            .map_err(|_| AppError::TxnError)?;
-        sqlx::query("SET LOCAL app.current_user_id = $1")
-            .bind(user_uid)
+        sqlx::query(set_local_user_id(user_uid))
            .execute(&mut *txn)
            .await
            .map_err(AppError::Database)?;
@@ -231,7 +241,7 @@ impl PrService {
        .fetch_optional(&mut *txn).await.map_err(AppError::Database)?
        .ok_or(AppError::NotFound("review not found or already submitted".into()))?;

-        if state == "approved" || state == "changes_requested" {
+        if state == ReviewState::Approved || state == ReviewState::ChangesRequested {
            sqlx::query(
                "UPDATE pr_status SET approvals_count = (SELECT COUNT(*) FROM pr_review r \
                 JOIN pull_request pr ON pr.id = r.pull_request_id \
@@ -274,8 +284,7 @@ impl PrService {
            .begin()
            .await
            .map_err(|_| AppError::TxnError)?;
-        sqlx::query("SET LOCAL app.current_user_id = $1")
-            .bind(user_uid)
+        sqlx::query(set_local_user_id(user_uid))
            .execute(&mut *txn)
            .await
            .map_err(AppError::Database)?;