implement since/before ordering for range requests

6 files changed, 131 insertions, 75 deletions

diff --git a/rust/src/database_models.rs b/rust/src/database_models.rs
index c033c6e5..63fbcb29 100644
--- a/rust/src/database_models.rs
+++ b/rust/src/database_models.rs
@@ -563,11 +563,12 @@ pub struct EditgroupRow {
 impl EditgroupRow {
     /// Returns an Editgroup API model *without* the entity edits actually populated. Useful for,
     /// eg, entity history queries (where we already have the entity edit we want)
-    pub fn into_model_partial(self) -> Editgroup {
+    pub fn into_model_partial(self, changelog_index: Option<i64>) -> Editgroup {
         Editgroup {
             editgroup_id: Some(uuid2fcid(&self.id)),
             editor_id: Some(uuid2fcid(&self.editor_id)),
             editor: None,
+            changelog_index: changelog_index,
             submitted: self
                 .submitted
                 .map(|v| chrono::DateTime::from_utc(v, chrono::Utc)),
diff --git a/rust/src/editing.rs b/rust/src/editing.rs
index 0d7a6b13..33caf6a9 100644
--- a/rust/src/editing.rs
+++ b/rust/src/editing.rs
@@ -50,6 +50,7 @@ pub fn make_edit_context(
                 editgroup_id: None,
                 editor_id: Some(editor_id.to_string()),
                 editor: None,
+                changelog_index: None,
                 submitted: None,
                 description: None,
                 extra: None,
diff --git a/rust/src/editing_crud.rs b/rust/src/editing_crud.rs
index d3782f7b..65403eee 100644
--- a/rust/src/editing_crud.rs
+++ b/rust/src/editing_crud.rs
@@ -63,6 +63,10 @@ impl EditorCrud for Editor {
 
 pub trait EditgroupCrud {
     fn db_get(conn: &DbConn, editgroup_id: FatcatId) -> Result<EditgroupRow>;
+    fn db_get_with_changelog(
+        conn: &DbConn,
+        editgroup_id: FatcatId,
+    ) -> Result<(EditgroupRow, Option<ChangelogRow>)>;
     fn db_expand(&mut self, conn: &DbConn, expand: ExpandFlags) -> Result<()>;
     fn db_get_range_for_editor(
         conn: &DbConn,
@@ -70,7 +74,7 @@ pub trait EditgroupCrud {
         limit: u64,
         since: Option<chrono::DateTime<chrono::Utc>>,
         before: Option<chrono::DateTime<chrono::Utc>>,
-    ) -> Result<Vec<EditgroupRow>>;
+    ) -> Result<Vec<(EditgroupRow, Option<ChangelogRow>)>>;
     fn db_get_range_reviewable(
         conn: &DbConn,
         limit: u64,
@@ -87,30 +91,35 @@ pub trait EditgroupCrud {
 }
 
 impl EditgroupCrud for Editgroup {
-    // XXX: this could *alwas* return changelog status as well. If we do that, can we get rid of
-    // the is_accepted thing? no, still want it as denormalized speed-up in some queries/filters.
-
     /// This method does *not* expand the 'edits'; currently that's still done in the endpoint
     /// handler, but it probably should be done in this trait with a db_expand()
     fn db_get(conn: &DbConn, editgroup_id: FatcatId) -> Result<EditgroupRow> {
-        let row: EditgroupRow = editgroup::table
-            .find(editgroup_id.to_uuid())
-            .get_result(conn)?;
+        // Note: at least for now, continue to fetch along with changelog to ensure is_accepted is
+        // consistent.
+        let (row, _): (EditgroupRow, Option<ChangelogRow>) =
+            Self::db_get_with_changelog(conn, editgroup_id)?;
+        Ok(row)
+    }
+
+    fn db_get_with_changelog(
+        conn: &DbConn,
+        editgroup_id: FatcatId,
+    ) -> Result<(EditgroupRow, Option<ChangelogRow>)> {
+        let (eg_row, cl_row): (EditgroupRow, Option<ChangelogRow>) = editgroup::table
+            .left_outer_join(changelog::table)
+            .filter(editgroup::id.eq(editgroup_id.to_uuid()))
+            .first(conn)?;
 
-        // Note: for now this is really conservative and verifies the is_accepted flag every time
-        let count: i64 = changelog::table
-            .filter(changelog::editgroup_id.eq(editgroup_id.to_uuid()))
-            .count()
-            .get_result(conn)?;
         ensure!(
-            (count > 0) == row.is_accepted,
+            cl_row.is_some() == eg_row.is_accepted,
             "internal database consistency error on editgroup: {}",
             editgroup_id
         );
-        Ok(row)
+        Ok((eg_row, cl_row))
     }
 
-    /// Note: this *still* doesn't epand the 'edits', at least yet.
+    /// Note: this *still* doesn't epand the 'edits', at least yet. *Only* the direct editgrop
+    /// 'GET' handler does that.
     fn db_expand(&mut self, conn: &DbConn, expand: ExpandFlags) -> Result<()> {
         if expand.editors {
             let editor_id = FatcatId::from_str(
@@ -129,17 +138,34 @@ impl EditgroupCrud for Editgroup {
         limit: u64,
         since: Option<chrono::DateTime<chrono::Utc>>,
         before: Option<chrono::DateTime<chrono::Utc>>,
-    ) -> Result<Vec<EditgroupRow>> {
-        // TODO: since/before
-        let rows: Vec<EditgroupRow> = match (since, before) {
-            _ => {
+    ) -> Result<Vec<(EditgroupRow, Option<ChangelogRow>)>> {
+        let rows: Vec<(EditgroupRow, Option<ChangelogRow>)> = match (since, before) {
+            (Some(since), None) => {
+                editgroup::table
+                    .left_outer_join(changelog::table)
+                    .filter(editgroup::editor_id.eq(editor_id.to_uuid()))
+                    .filter(editgroup::created.gt(since))
+                    .order_by(editgroup::created.asc())
+                    .limit(limit as i64)
+                    .get_results(conn)?
+            },
+            (_, Some(before)) => {
                 editgroup::table
+                    .left_outer_join(changelog::table)
                     .filter(editgroup::editor_id.eq(editor_id.to_uuid()))
-                    // XXX: .filter(editgroup::created
+                    .filter(editgroup::created.lt(before))
                     .order_by(editgroup::created.desc())
                     .limit(limit as i64)
                     .get_results(conn)?
-            }
+            },
+            (None, None) => {
+                editgroup::table
+                    .left_outer_join(changelog::table)
+                    .filter(editgroup::editor_id.eq(editor_id.to_uuid()))
+                    .order_by(editgroup::created.desc())
+                    .limit(limit as i64)
+                    .get_results(conn)?
+            },
         };
         Ok(rows)
     }
@@ -150,14 +176,33 @@ impl EditgroupCrud for Editgroup {
         since: Option<chrono::DateTime<chrono::Utc>>,
         before: Option<chrono::DateTime<chrono::Utc>>,
     ) -> Result<Vec<EditgroupRow>> {
-        // TODO: since/before
         let rows: Vec<EditgroupRow> = match (since, before) {
-            _ => editgroup::table
-                .filter(editgroup::is_accepted.eq(false))
-                .filter(editgroup::submitted.is_not_null())
-                .order_by(editgroup::created.desc())
-                .limit(limit as i64)
-                .get_results(conn)?,
+            (Some(since), None) => {
+                editgroup::table
+                    .filter(editgroup::is_accepted.eq(false))
+                    .filter(editgroup::submitted.is_not_null())
+                    .filter(editgroup::submitted.gt(since))
+                    .order_by(editgroup::submitted.asc())
+                    .limit(limit as i64)
+                    .get_results(conn)?
+            },
+            (_, Some(before)) => {
+                editgroup::table
+                    .filter(editgroup::is_accepted.eq(false))
+                    .filter(editgroup::submitted.is_not_null())
+                    .filter(editgroup::submitted.lt(before))
+                    .order_by(editgroup::submitted.desc())
+                    .limit(limit as i64)
+                    .get_results(conn)?
+            },
+            (None, None) => {
+                editgroup::table
+                    .filter(editgroup::is_accepted.eq(false))
+                    .filter(editgroup::submitted.is_not_null())
+                    .order_by(editgroup::created.desc())
+                    .limit(limit as i64)
+                    .get_results(conn)?
+            },
         };
         Ok(rows)
     }
@@ -269,13 +314,30 @@ impl EditgroupAnnotationCrud for EditgroupAnnotation {
         since: Option<chrono::DateTime<chrono::Utc>>,
         before: Option<chrono::DateTime<chrono::Utc>>,
     ) -> Result<Vec<EditgroupAnnotationRow>> {
-        // TODO: since/before
         let rows: Vec<EditgroupAnnotationRow> = match (since, before) {
-            _ => editgroup_annotation::table
-                .filter(editgroup_annotation::editor_id.eq(editor_id.to_uuid()))
-                .order_by(editgroup_annotation::created.desc())
-                .limit(limit as i64)
-                .get_results(conn)?,
+            (Some(since), None) => {
+                 editgroup_annotation::table
+                    .filter(editgroup_annotation::editor_id.eq(editor_id.to_uuid()))
+                    .filter(editgroup_annotation::created.gt(since))
+                    .order_by(editgroup_annotation::created.asc())
+                    .limit(limit as i64)
+                    .get_results(conn)?
+            },
+            (_, Some(before)) => {
+                 editgroup_annotation::table
+                    .filter(editgroup_annotation::editor_id.eq(editor_id.to_uuid()))
+                    .filter(editgroup_annotation::created.lt(before))
+                    .order_by(editgroup_annotation::created.desc())
+                    .limit(limit as i64)
+                    .get_results(conn)?
+            },
+            (None, None) => {
+                 editgroup_annotation::table
+                    .filter(editgroup_annotation::editor_id.eq(editor_id.to_uuid()))
+                    .order_by(editgroup_annotation::created.desc())
+                    .limit(limit as i64)
+                    .get_results(conn)?
+            },
         };
         Ok(rows)
     }
@@ -287,13 +349,30 @@ impl EditgroupAnnotationCrud for EditgroupAnnotation {
         since: Option<chrono::DateTime<chrono::Utc>>,
         before: Option<chrono::DateTime<chrono::Utc>>,
     ) -> Result<Vec<EditgroupAnnotationRow>> {
-        // TODO: since/before
         let rows: Vec<EditgroupAnnotationRow> = match (since, before) {
-            _ => editgroup_annotation::table
-                .filter(editgroup_annotation::editgroup_id.eq(editgroup_id.to_uuid()))
-                .order_by(editgroup_annotation::created.desc())
-                .limit(limit as i64)
-                .get_results(conn)?,
+            (Some(since), None) => {
+                 editgroup_annotation::table
+                    .filter(editgroup_annotation::editgroup_id.eq(editgroup_id.to_uuid()))
+                    .filter(editgroup_annotation::created.gt(since))
+                    .order_by(editgroup_annotation::created.asc())
+                    .limit(limit as i64)
+                    .get_results(conn)?
+            },
+            (_, Some(before)) => {
+                 editgroup_annotation::table
+                    .filter(editgroup_annotation::editgroup_id.eq(editgroup_id.to_uuid()))
+                    .filter(editgroup_annotation::created.lt(before))
+                    .order_by(editgroup_annotation::created.desc())
+                    .limit(limit as i64)
+                    .get_results(conn)?
+            },
+            (None, None) => {
+                 editgroup_annotation::table
+                    .filter(editgroup_annotation::editgroup_id.eq(editgroup_id.to_uuid()))
+                    .order_by(editgroup_annotation::created.desc())
+                    .limit(limit as i64)
+                    .get_results(conn)?
+            },
         };
         Ok(rows)
     }
diff --git a/rust/src/endpoint_handlers.rs b/rust/src/endpoint_handlers.rs
index dcbb3d90..bc606af9 100644
--- a/rust/src/endpoint_handlers.rs
+++ b/rust/src/endpoint_handlers.rs
@@ -386,7 +386,7 @@ impl Server {
         editgroup: models::Editgroup,
     ) -> Result<Editgroup> {
         let row = editgroup.db_create(conn, false)?;
-        Ok(row.into_model_partial())
+        Ok(row.into_model_partial(None))
     }
 
     pub fn get_editgroup_handler(
@@ -394,8 +394,8 @@ impl Server {
         conn: &DbConn,
         editgroup_id: FatcatId,
     ) -> Result<Editgroup> {
-        let row: EditgroupRow = Editgroup::db_get(conn, editgroup_id)?;
-        let mut editgroup = row.into_model_partial();
+        let (eg_row, cl_row) = Editgroup::db_get_with_changelog(conn, editgroup_id)?;
+        let mut editgroup = eg_row.into_model_partial(cl_row.map(|cl| cl.id));
 
         let edits = EditgroupEdits {
             containers: Some(
@@ -465,31 +465,6 @@ impl Server {
         Ok(row.into_model())
     }
 
-    pub fn get_editor_changelog_handler(
-        &self,
-        conn: &DbConn,
-        editor_id: FatcatId,
-    ) -> Result<Vec<ChangelogEntry>> {
-        // XXX: delete me?
-        // TODO: single query
-        let editor: EditorRow = Editor::db_get(&conn, editor_id)?;
-        let changes: Vec<(ChangelogRow, EditgroupRow)> = changelog::table
-            .inner_join(editgroup::table)
-            .filter(editgroup::editor_id.eq(editor.id))
-            .load(conn)?;
-
-        let entries = changes
-            .into_iter()
-            .map(|(cl_row, eg_row)| ChangelogEntry {
-                index: cl_row.id,
-                editgroup: Some(eg_row.into_model_partial()),
-                editgroup_id: uuid2fcid(&cl_row.editgroup_id),
-                timestamp: chrono::DateTime::from_utc(cl_row.timestamp, chrono::Utc),
-            })
-            .collect();
-        Ok(entries)
-    }
-
     pub fn get_changelog_handler(
         &self,
         conn: &DbConn,
@@ -507,7 +482,7 @@ impl Server {
             .into_iter()
             .map(|(cl_row, eg_row)| ChangelogEntry {
                 index: cl_row.id,
-                editgroup: Some(eg_row.into_model_partial()),
+                editgroup: Some(eg_row.into_model_partial(None)),
                 editgroup_id: uuid2fcid(&cl_row.editgroup_id),
                 timestamp: chrono::DateTime::from_utc(cl_row.timestamp, chrono::Utc),
             })
diff --git a/rust/src/endpoints.rs b/rust/src/endpoints.rs
index 77f70ce0..33c4bc3e 100644
--- a/rust/src/endpoints.rs
+++ b/rust/src/endpoints.rs
@@ -752,9 +752,9 @@ impl Api for Server {
             .transaction(|| {
                 let editor_id = FatcatId::from_str(&editor_id)?;
                 let limit = cmp::min(100, limit.unwrap_or(20)) as u64;
-                let row =
+                let rows =
                     Editgroup::db_get_range_for_editor(&conn, editor_id, limit, since, before)?;
-                Ok(row.into_iter().map(|eg| eg.into_model_partial()).collect())
+                Ok(rows.into_iter().map(|(eg, cl)| eg.into_model_partial(cl.map(|v| v.id))).collect())
             })
             .map_err(|e: Error| FatcatError::from(e))
         {
@@ -893,7 +893,7 @@ impl Api for Server {
                 let limit = cmp::min(100, limit.unwrap_or(20)) as u64;
                 let row = Editgroup::db_get_range_reviewable(&conn, limit, since, before)?;
                 let mut editgroups: Vec<Editgroup> =
-                    row.into_iter().map(|eg| eg.into_model_partial()).collect();
+                    row.into_iter().map(|eg| eg.into_model_partial(None)).collect();
                 if let Some(expand) = expand {
                     let expand = ExpandFlags::from_str(&expand)?;
                     for eg in editgroups.iter_mut() {
@@ -991,7 +991,7 @@ impl Api for Server {
                 };
                 editgroup
                     .db_update(&conn, editgroup_id, submit)
-                    .map(|eg| eg.into_model_partial())
+                    .map(|eg| eg.into_model_partial(None)) // can't update an accepted editgroup
             })
             .map_err(|e: Error| FatcatError::from(e))
         {
diff --git a/rust/src/entity_crud.rs b/rust/src/entity_crud.rs
index 0b70d417..ce1c1ed7 100644
--- a/rust/src/entity_crud.rs
+++ b/rust/src/entity_crud.rs
@@ -550,7 +550,7 @@ macro_rules! generic_db_get_history {
                 .map(|(eg_row, cl_row, e_row)| {
                     Ok(EntityHistoryEntry {
                         edit: e_row.into_model()?,
-                        editgroup: eg_row.into_model_partial(),
+                        editgroup: eg_row.into_model_partial(None),
                         changelog_entry: cl_row.into_model(),
                     })
                 })