Fetch images from artifact ids and render their captions correctly in…

… listing page (#211) * Add route to fetch artifacts in bulk and return only images * In Listings, fetch images from artifact_ids This fixes captions being just "caption" (which was a placeholder waiting for this commit) and also fixes trying to render URDF tar gunzips. * Add 'name' field to Artifacts, which stores filename This is so people have a good identifier for URDFs, etc. * Take in a listing for upload_urdf - Check whether user actually owns that listing - Update listing artifact_ids * rename batch to get_batch * Change route that gets unique urdf to route that gets latest
kscalelabs · Jul 30, 2024 · e443b73 · e443b73
1 parent 795ec8e
commit e443b73
Show file tree

Hide file tree

Showing 9 changed files with 138 additions and 83 deletions.
diff --git a/frontend/src/components/files/ViewImage.tsx b/frontend/src/components/files/ViewImage.tsx
@@ -4,7 +4,7 @@ import React from "react";
 interface ImageProps {
   imageId: string;
   size: "small" | "large";
-  caption: string;
+  caption?: string;
 }
 
 const ImageComponent: React.FC<ImageProps> = ({ imageId, size, caption }) => {

diff --git a/frontend/src/hooks/api.tsx b/frontend/src/hooks/api.tsx
@@ -191,6 +191,15 @@ export class api {
     });
   }
 
+  public async getImages(ids: string[]): Promise<Artifact[]> {
+    return this.callWrapper(async () => {
+      const response = await this.api.get("/images/batch", {
+        params: { ids: ids.join(",") },
+      });
+      return response.data;
+    });
+  }
+
   public async uploadImage(formData: FormData): Promise<string> {
     return this.callWrapper(async () => {
       const res = await this.api.post<UploadImageResponse>(

diff --git a/frontend/src/pages/ListingDetails.tsx b/frontend/src/pages/ListingDetails.tsx
@@ -2,7 +2,7 @@ import TCButton from "components/files/TCButton";
 import ImageComponent from "components/files/ViewImage";
 import { humanReadableError } from "constants/backend";
 import { useAlertQueue } from "hooks/alerts";
-import { api } from "hooks/api";
+import { api, Artifact } from "hooks/api";
 import { useAuthentication } from "hooks/auth";
 import { useEffect, useState } from "react";
 import {
@@ -47,9 +47,10 @@ const RenderListing = ({
   const [userId, setUserId] = useState<string | null>(null);
   const [show, setShow] = useState(false);
   const [ownerEmail, setOwnerEmail] = useState<string | null>(null);
-  const [imageIndex, setArtifactIndex] = useState(0);
+  const [imageIndex, setArtifactIndex] = useState<number>(0);
   const [showDelete, setShowDelete] = useState(false);
   const [children, setChildren] = useState<Child[] | null>(null);
+  const [images, setImages] = useState<Artifact[]>([]);
 
   const handleClose = () => setShow(false);
   const handleShow = () => setShow(true);
@@ -77,6 +78,8 @@ const RenderListing = ({
       try {
         const ownerEmail = await auth_api.getUserById(user_id);
         setOwnerEmail(ownerEmail);
+        const images = await auth_api.getImages(artifact_ids);
+        setImages(images);
       } catch (err) {
         addAlert(humanReadableError(err), "error");
       }
@@ -229,9 +232,9 @@ const RenderListing = ({
             data-bs-theme="dark"
             style={{ border: "1px solid #ccc" }}
             interval={null}
-            controls={artifact_ids.length > 1}
+            controls={images.length > 1}
           >
-            {artifact_ids.map((id, key) => (
+            {images.map((image, key) => (
               <Carousel.Item key={key}>
                 <div
                   style={{
@@ -245,79 +248,82 @@ const RenderListing = ({
                   }}
                 >
                   <ImageComponent
-                    imageId={id}
+                    imageId={image.id}
                     size={"large"}
-                    caption={"caption"}
+                    caption={image.caption}
                   />
                 </div>
-                <Carousel.Caption
-                  style={{
-                    backgroundColor: "rgba(255, 255, 255, 0.5)",
-                    color: "black",
-                    padding: "0.1rem",
-                    // Put the caption at the top
-                    top: 10,
-                    bottom: "unset",
-                  }}
-                >
-                  {"caption"}
-                </Carousel.Caption>
+                {image.caption && (
+                  <Carousel.Caption
+                    style={{
+                      backgroundColor: "rgba(255, 255, 255, 0.5)",
+                      color: "black",
+                      padding: "0.1rem",
+                      // Put the caption at the top
+                      top: 10,
+                      bottom: "unset",
+                    }}
+                  >
+                    {image.caption}
+                  </Carousel.Caption>
+                )}
               </Carousel.Item>
             ))}
           </Carousel>
         </Col>
       </Row>
 
-      <Modal
-        show={show}
-        onHide={handleClose}
-        fullscreen="md-down"
-        centered
-        size="lg"
-        scrollable
-      >
-        <Modal.Header closeButton>
-          <Modal.Title>
-            {/* TO-DO: This supposed to be the caption */}
-            {artifact_ids[imageIndex]} ({imageIndex + 1} of{" "}
-            {artifact_ids.length})
-          </Modal.Title>
-        </Modal.Header>
-        <Modal.Body>
-          <div style={{ display: "flex", justifyContent: "center" }}>
-            <ImageComponent
-              imageId={artifact_ids[imageIndex]}
-              size={"large"}
-              caption={artifact_ids[imageIndex]}
-            />
-          </div>
-        </Modal.Body>
-        <Modal.Footer>
-          {artifact_ids.length > 1 && (
-            <ButtonGroup>
-              <TCButton
-                variant="primary"
-                onClick={() => {
-                  setArtifactIndex(
-                    (imageIndex - 1 + artifact_ids.length) %
-                      artifact_ids.length,
-                  );
-                }}
-              >
-                Previous
-              </TCButton>
-              <TCButton
-                variant="primary"
-                onClick={() => {
-                  setArtifactIndex((imageIndex + 1) % artifact_ids.length);
-                }}
-              >
-                Next
-              </TCButton>
-            </ButtonGroup>
-          )}
-        </Modal.Footer>
-      </Modal>
+      {images.length > 0 && (
+        <Modal
+          show={show}
+          onHide={handleClose}
+          fullscreen="md-down"
+          centered
+          size="lg"
+          scrollable
+        >
+          <Modal.Header closeButton>
+            <Modal.Title>
+              {images[imageIndex].caption} ({imageIndex + 1} of {images.length})
+            </Modal.Title>
+          </Modal.Header>
+          <Modal.Body>
+            <div style={{ display: "flex", justifyContent: "center" }}>
+              <ImageComponent
+                imageId={images[imageIndex].id}
+                size={"large"}
+                {...(images[imageIndex].caption && {
+                  caption: images[imageIndex].caption,
+                })}
+              />
+            </div>
+          </Modal.Body>
+          <Modal.Footer>
+            {images.length > 1 && (
+              <ButtonGroup>
+                <TCButton
+                  variant="primary"
+                  onClick={() => {
+                    setArtifactIndex(
+                      (imageIndex - 1 + images.length) % images.length,
+                    );
+                  }}
+                >
+                  Previous
+                </TCButton>
+                <TCButton
+                  variant="primary"
+                  onClick={() => {
+                    setArtifactIndex((imageIndex + 1) % images.length);
+                  }}
+                >
+                  Next
+                </TCButton>
+              </ButtonGroup>
+            )}
+          </Modal.Footer>
+        </Modal>
+      )}
     </>
   );
 };

diff --git a/store/app/crud/artifacts.py b/store/app/crud/artifacts.py
@@ -65,12 +65,14 @@ async def _upload_cropped_image(self, image: Image.Image, image_id: str, size: A
 
     async def upload_image(
         self,
+        name: str,
         image: Image.Image,
         user_id: str,
         description: str | None = None,
     ) -> Artifact:
         artifact = Artifact.create(
             user_id=user_id,
+            name=name,
             artifact_type="image",
             sizes=list(SizeMapping.keys()),
             description=description,
@@ -90,12 +92,14 @@ async def upload_image(
 
     async def upload_urdf(
         self,
+        name: str,
         file: io.BytesIO | BinaryIO,
         user_id: str,
         description: str | None = None,
     ) -> Artifact:
         artifact = Artifact.create(
             user_id=user_id,
+            name=name,
             artifact_type="urdf",
             description=description,
         )
@@ -107,12 +111,14 @@ async def upload_urdf(
 
     async def upload_mjcf(
         self,
+        name: str,
         file: io.BytesIO | BinaryIO,
         user_id: str,
         description: str | None = None,
     ) -> Artifact:
         artifact = Artifact.create(
             user_id=user_id,
+            name=name,
             artifact_type="mjcf",
             description=description,
         )

diff --git a/store/app/crud/listings.py b/store/app/crud/listings.py
@@ -3,7 +3,7 @@
 import asyncio
 import logging
 
-from store.app.crud.base import BaseCrud, InternalError, ItemNotFoundError
+from store.app.crud.base import BaseCrud, ItemNotFoundError
 from store.app.model import Artifact, Listing, ListingTag
 
 logger = logging.getLogger(__name__)
@@ -54,7 +54,7 @@ async def delete_tag(self, listing_id: str, tag: str) -> None:
         listing_tag = ListingTag.create(listing_id=listing_id, tag=tag)
         return await self._delete_item(listing_tag)
 
-    async def get_urdf_id(
+    async def get_latest_urdf_id(
         self,
         listing_id: str,
     ) -> str | None:
@@ -63,9 +63,4 @@ async def get_urdf_id(
         urdfs = [artifact for artifact in artifacts if artifact.artifact_type == "urdf"]
         if len(urdfs) == 0:
             return None
-        if len(urdfs) > 1:
-            raise InternalError(
-                f"""More than one URDF found for listing {listing_id}.
-                This is due to incorrect data in the database, likely caused by a bug."""
-            )
-        return urdfs[0].id
+        return max(urdfs, key=lambda urdf: urdf.timestamp)
diff --git a/store/app/model.py b/store/app/model.py
@@ -5,6 +5,7 @@
 expects (for example, converting a UUID into a string).
 """
 
+import time
 from datetime import datetime, timedelta
 from typing import Literal, Self
 
@@ -99,24 +100,29 @@ class Artifact(RobolistBaseModel):
     """
 
     user_id: str
+    name: str
     artifact_type: ArtifactType
     sizes: list[ArtifactSize] | None = None
     description: str | None = None
+    timestamp: int
 
     @classmethod
     def create(
         cls,
         user_id: str,
+        name: str,
         artifact_type: ArtifactType,
         sizes: list[ArtifactSize] | None = None,
         description: str | None = None,
     ) -> Self:
         return cls(
             id=str(new_uuid()),
+            name=name,
             user_id=user_id,
             artifact_type=artifact_type,
             sizes=sizes,
             description=description,
+            timestamp=int(time.time()),
         )
 
 

diff --git a/store/app/routers/image.py b/store/app/routers/image.py
@@ -3,14 +3,14 @@
 import logging
 from typing import Annotated
 
-from fastapi import APIRouter, Depends, HTTPException, UploadFile, status
+from fastapi import APIRouter, Depends, HTTPException, Query, UploadFile, status
 from fastapi.responses import RedirectResponse
 from PIL import Image
 from pydantic.main import BaseModel
 
 from store.app.crud.artifacts import get_image_name
 from store.app.db import Crud
-from store.app.model import ArtifactSize, User
+from store.app.model import Artifact, ArtifactSize, User
 from store.app.routers.users import get_session_user_with_write_permission
 from store.settings import settings
 
@@ -50,6 +50,7 @@ async def upload_image(
         image = Image.open(file.file)
         artifact = await crud.upload_image(
             image=image,
+            name=file.filename,
             user_id=user.id,
             description=description,
         )
@@ -64,3 +65,21 @@ async def image_url(image_id: str, size: ArtifactSize) -> RedirectResponse:
     # TODO: Use CloudFront API to return a signed CloudFront URL.
     image_url = f"{settings.site.artifact_base_url}/{get_image_name(image_id, size)}"
     return RedirectResponse(url=image_url)
+
+
+class ImageInfoResponse(BaseModel):
+    id: str
+    caption: str | None
+
+
+@image_router.get("/batch")
+async def batch(
+    crud: Annotated[Crud, Depends(Crud.get)],
+    ids: list[str] = Query(description="List of part ids"),
+) -> list[ImageInfoResponse]:
+    artifacts = await crud._get_item_batch(ids, Artifact)
+    return [
+        ImageInfoResponse(id=artifact.id, caption=artifact.description)
+        for artifact in artifacts
+        if artifact.artifact_type == "image"
+    ]
diff --git a/store/app/routers/listings.py b/store/app/routers/listings.py
@@ -36,7 +36,7 @@ async def list_listings(
 
 
 @listings_router.get("/batch")
-async def batch(
+async def get_batch(
     crud: Annotated[Crud, Depends(Crud.get)],
     ids: list[str] = Query(description="List of part ids"),
 ) -> list[Listing]: