From 1d6993b8dcc711ea3c33675b750ba6fa0fafdc5d Mon Sep 17 00:00:00 2001
From: Jiajie Chen <c@jia.je>
Date: Thu, 14 Mar 2024 15:26:48 +0800
Subject: [PATCH] feat: add sorting to jobs

---
 frontend/src/pages/jobs/index.vue | 27 +++++++++++---
 server/src/api.rs                 | 16 ++++----
 server/src/bot.rs                 | 12 +++---
 server/src/github.rs              |  2 +-
 server/src/routes/job.rs          | 62 +++++++++++++++++++++++--------
 server/src/routes/pipeline.rs     |  2 +-
 server/src/routes/worker.rs       | 16 +++-----
 worker/src/build.rs               |  2 +-
 8 files changed, 89 insertions(+), 50 deletions(-)

diff --git a/frontend/src/pages/jobs/index.vue b/frontend/src/pages/jobs/index.vue
index 326ba97..903d9b2 100644
--- a/frontend/src/pages/jobs/index.vue
+++ b/frontend/src/pages/jobs/index.vue
@@ -8,6 +8,7 @@
           :items="serverItems"
           :items-length="totalItems"
           :loading="loading"
+          :sort-by="sortBy"
           item-value="id"
           @update:options="loadItems">
           <template #item.id="{ item }">
@@ -29,9 +30,15 @@
   import axios from 'axios';
   import { hostname } from '@/common';
 
+  interface SortBy {
+    key: string;
+    order: string;
+  }
+
   interface LoadItemsOpts {
     page: number;
     itemsPerPage: number;
+    sortBy: SortBy[];
   }
 
   interface Job {
@@ -42,19 +49,27 @@
     data: () => ({
       itemsPerPage: 10,
       headers: [
-        { title: 'Job ID', key: 'id', sortable: false },
-        { title: 'Pipeline ID', key: 'pipeline_id', sortable: false },
-        { title: 'Packages', key: 'packages', sortable: false },
-        { title: 'Architecture', key: 'arch', sortable: false },
+        { title: 'Job ID', key: 'id' },
+        { title: 'Pipeline ID', key: 'pipeline_id' },
+        { title: 'Packages', key: 'packages' },
+        { title: 'Architecture', key: 'arch' },
       ],
       loading: true,
       totalItems: 0,
-      serverItems: []
+      serverItems: [],
+      sortBy: [{
+        key: 'id',
+        order: 'desc'
+      }]
     }),
     methods: {
       async loadItems (opts: LoadItemsOpts) {
         this.loading = true;
-        let data = (await axios.get(hostname + `/api/job/list?page=${opts.page}&items_per_page=${opts.itemsPerPage}`)).data;
+        let url = hostname + `/api/job/list?page=${opts.page}&items_per_page=${opts.itemsPerPage}`;
+        if (opts.sortBy.length > 0) {
+          url += `&sort_key=${opts.sortBy[0].key}&sort_order=${opts.sortBy[0].order}`;
+        }
+        let data = (await axios.get(url)).data;
         this.totalItems = data.total_items;
         this.serverItems = data.items;
         this.loading = false;
diff --git a/server/src/api.rs b/server/src/api.rs
index fdf9f23..8d5aa56 100644
--- a/server/src/api.rs
+++ b/server/src/api.rs
@@ -31,7 +31,7 @@ pub async fn pipeline_new(
     source: &JobSource,
 ) -> anyhow::Result<Pipeline> {
     // sanitize archs arg
-    let mut archs: Vec<&str> = archs.split(",").collect();
+    let mut archs: Vec<&str> = archs.split(',').collect();
     archs.sort();
     archs.dedup();
     if archs.contains(&"noarch") && archs.len() > 1 {
@@ -108,7 +108,7 @@ pub async fn pipeline_new(
         creation_time: chrono::Utc::now(),
         source: source.to_string(),
         github_pr: github_pr.map(|pr| pr as i64),
-        telegram_user: telegram_user.map(|id| *id),
+        telegram_user: telegram_user.copied(),
     };
     let pipeline = diesel::insert_into(pipelines::table)
         .values(&new_pipeline)
@@ -217,22 +217,20 @@ pub async fn pipeline_new_pr(
                 pipeline_new(
                     pool,
                     git_branch,
-                    Some(&git_sha),
+                    Some(git_sha),
                     Some(pr.number),
                     &packages.join(","),
                     &archs,
-                    &source,
+                    source,
                 )
                 .await
             } else {
-                return Err(anyhow!(
+                Err(anyhow!(
                     "Please list packages to build in pr info starting with '#buildit'"
-                ));
+                ))
             }
         }
-        Err(err) => {
-            return Err(anyhow!("Failed to get pr info: {err}"));
-        }
+        Err(err) => Err(anyhow!("Failed to get pr info: {err}")),
     }
 }
 
diff --git a/server/src/bot.rs b/server/src/bot.rs
index 47df00d..6795779 100644
--- a/server/src/bot.rs
+++ b/server/src/bot.rs
@@ -127,8 +127,8 @@ async fn pipeline_new_and_report(
                     pipeline.id,
                     &pipeline.git_branch,
                     pipeline.github_pr.map(|n| n as u64),
-                    &pipeline.archs.split(",").collect::<Vec<_>>(),
-                    &pipeline.packages.split(",").collect::<Vec<_>>(),
+                    &pipeline.archs.split(',').collect::<Vec<_>>(),
+                    &pipeline.packages.split(',').collect::<Vec<_>>(),
                 ),
             )
             .parse_mode(ParseMode::Html)
@@ -194,7 +194,7 @@ pub async fn answer(bot: Bot, msg: Message, cmd: Command, pool: DbPool) -> Respo
                         pool.clone(),
                         pr_number,
                         archs,
-                        &JobSource::Telegram(msg.chat.id.0 as i64),
+                        &JobSource::Telegram(msg.chat.id.0),
                     )
                     .await
                     {
@@ -205,8 +205,8 @@ pub async fn answer(bot: Bot, msg: Message, cmd: Command, pool: DbPool) -> Respo
                                     pipeline.id,
                                     &pipeline.git_branch,
                                     pipeline.github_pr.map(|n| n as u64),
-                                    &pipeline.archs.split(",").collect::<Vec<_>>(),
-                                    &pipeline.packages.split(",").collect::<Vec<_>>(),
+                                    &pipeline.archs.split(',').collect::<Vec<_>>(),
+                                    &pipeline.packages.split(',').collect::<Vec<_>>(),
                                 ),
                             )
                             .parse_mode(ParseMode::Html)
@@ -553,7 +553,7 @@ pub async fn answer(bot: Bot, msg: Message, cmd: Command, pool: DbPool) -> Respo
                                     pool.clone(),
                                     "stable",
                                     &pkg.name,
-                                    &arch,
+                                    arch,
                                     &msg,
                                 )
                                 .await?;
diff --git a/server/src/github.rs b/server/src/github.rs
index 01a0854..9035656 100644
--- a/server/src/github.rs
+++ b/server/src/github.rs
@@ -88,5 +88,5 @@ pub async fn get_crab_github_installation() -> anyhow::Result<Option<Octocrab>>
             ));
         }
     }
-    return Ok(None);
+    Ok(None)
 }
diff --git a/server/src/routes/job.rs b/server/src/routes/job.rs
index 248d32f..27a02d7 100644
--- a/server/src/routes/job.rs
+++ b/server/src/routes/job.rs
@@ -1,19 +1,19 @@
-use crate::models::{Job, Pipeline, Worker};
+use crate::models::{Job, Pipeline};
 use crate::routes::{AnyhowError, AppState};
-
-use anyhow::Context;
+use crate::schema::jobs::BoxedQuery;
+use anyhow::{bail, Context};
 use axum::extract::{Json, Query, State};
-
-use diesel::{Connection, ExpressionMethods, OptionalExtension, QueryDsl, RunQueryDsl};
-
+use diesel::pg::Pg;
+use diesel::query_builder::QueryFragment;
+use diesel::{AppearsOnTable, Connection, ExpressionMethods, QueryDsl, RunQueryDsl};
 use serde::{Deserialize, Serialize};
 
-use teloxide::prelude::*;
-
 #[derive(Deserialize)]
 pub struct JobListRequest {
     page: i64,
     items_per_page: i64,
+    sort_key: Option<String>,
+    sort_order: Option<String>,
 }
 
 #[derive(Serialize)]
@@ -31,6 +31,22 @@ pub struct JobListResponse {
     items: Vec<JobListResponseItem>,
 }
 
+// https://stackoverflow.com/questions/59291037/how-do-i-conditionally-order-by-a-column-based-on-a-dynamic-parameter-with-diese
+fn sort_by_column<U: 'static>(
+    query: BoxedQuery<'static, Pg>,
+    column: U,
+    sort_order: &str,
+) -> anyhow::Result<BoxedQuery<'static, Pg>>
+where
+    U: ExpressionMethods + QueryFragment<Pg> + AppearsOnTable<crate::schema::jobs::table> + Send,
+{
+    Ok(match sort_order {
+        "asc" => query.order_by(column.asc()),
+        "desc" => query.order_by(column.desc()),
+        _ => bail!("Invalid sort_order"),
+    })
+}
+
 pub async fn job_list(
     Query(query): Query<JobListRequest>,
     State(AppState { pool, .. }): State<AppState>,
@@ -40,17 +56,33 @@ pub async fn job_list(
         .context("Failed to get db connection from pool")?;
 
     Ok(Json(
-        conn.transaction::<JobListResponse, diesel::result::Error, _>(|conn| {
+        conn.transaction::<JobListResponse, anyhow::Error, _>(|conn| {
             let total_items = crate::schema::jobs::dsl::jobs.count().get_result(conn)?;
 
+            let sql = crate::schema::jobs::dsl::jobs.into_boxed();
+
+            let sort_key = query.sort_key.as_ref().map(String::as_str).unwrap_or("id");
+            let sort_order = query
+                .sort_order
+                .as_ref()
+                .map(String::as_str)
+                .unwrap_or("desc");
+            let sql = match sort_key {
+                "id" => sort_by_column(sql, crate::schema::jobs::dsl::id, sort_order)?,
+                "pipeline_id" => {
+                    sort_by_column(sql, crate::schema::jobs::dsl::pipeline_id, sort_order)?
+                }
+                "packages" => sort_by_column(sql, crate::schema::jobs::dsl::packages, sort_order)?,
+                "arch" => sort_by_column(sql, crate::schema::jobs::dsl::arch, sort_order)?,
+                _ => {
+                    bail!("Invalid sort_key");
+                }
+            };
+
             let jobs = if query.items_per_page == -1 {
-                crate::schema::jobs::dsl::jobs
-                    .order_by(crate::schema::jobs::dsl::id)
-                    .load::<Job>(conn)?
+                sql.load::<Job>(conn)?
             } else {
-                crate::schema::jobs::dsl::jobs
-                    .order_by(crate::schema::jobs::dsl::id)
-                    .offset((query.page - 1) * query.items_per_page)
+                sql.offset((query.page - 1) * query.items_per_page)
                     .limit(query.items_per_page)
                     .load::<Job>(conn)?
             };
diff --git a/server/src/routes/pipeline.rs b/server/src/routes/pipeline.rs
index 0dfec36..240b44d 100644
--- a/server/src/routes/pipeline.rs
+++ b/server/src/routes/pipeline.rs
@@ -55,7 +55,7 @@ pub async fn pipeline_new_pr(
     let pipeline = api::pipeline_new_pr(
         pool,
         payload.pr,
-        payload.archs.as_ref().map(|s| s.as_str()),
+        payload.archs.as_deref(),
         &JobSource::Manual,
     )
     .await?;
diff --git a/server/src/routes/worker.rs b/server/src/routes/worker.rs
index 8f264c8..2763b29 100644
--- a/server/src/routes/worker.rs
+++ b/server/src/routes/worker.rs
@@ -316,9 +316,9 @@ pub async fn handle_success_message(
             if pipeline.source == "telegram" {
                 if let Some(bot) = bot {
                     let s = to_html_build_result(
-                        &pipeline,
-                        &job,
-                        &job_ok,
+                        pipeline,
+                        job,
+                        job_ok,
                         &req.hostname,
                         &req.arch,
                         success,
@@ -340,14 +340,8 @@ pub async fn handle_success_message(
             }
 
             // if associated with github pr, update comments
-            let new_content = to_markdown_build_result(
-                &pipeline,
-                &job,
-                &job_ok,
-                &req.hostname,
-                &req.arch,
-                success,
-            );
+            let new_content =
+                to_markdown_build_result(pipeline, job, job_ok, &req.hostname, &req.arch, success);
             if let Some(pr_num) = pipeline.github_pr {
                 let crab = match octocrab::Octocrab::builder()
                     .user_access_token(ARGS.github_access_token.clone())
diff --git a/worker/src/build.rs b/worker/src/build.rs
index 9509e03..8565ef9 100644
--- a/worker/src/build.rs
+++ b/worker/src/build.rs
@@ -150,7 +150,7 @@ async fn build(
 
             // build packages
             let mut ciel_args = vec!["build", "-i", &args.ciel_instance];
-            ciel_args.extend(job.packages.split(","));
+            ciel_args.extend(job.packages.split(','));
             let output = get_output_logged("ciel", &ciel_args, &args.ciel_path, &mut logs).await?;
 
             success = output.status.success();