Sk.cl.upload site image #295

CiciLing · 2023-10-22T19:54:18Z

Checklist

1. Run yarn run check
2. Run yarn run test

Why

Resolves #241

This ticket allows the end users to upload their tree image to their adopted tree. Previously, there was no way of uploading tree images through the site and thus this will come in handy to users who want to upload their tree photos to share with other users.

This PR

This change adds an upload image component to the tree pages. Users are able to upload tree images that are converted to b64. The upload image component with its API client 'uploadImage' posts the uploaded image and siteEntryId to /api/v1/protected/sites/site_image/${siteEntryId}. Note that you have to adopt the tree to be able to upload a tree image.

Screenshots

Screen recording

Screen.Recording.2023-10-22.at.3.56.59.PM.mov

Verification Steps

To check if this upload component works, try uploading an image and go to AWS to see if the image shows up in folder 'sfft-user-upload' - (the content you defined in 'aws_s3_upload_dir' in server.properties from the backend). Additionally, there are three tests we could run in protectedApiClient.test to test upload image API client.

- Add basic text - Add component definition - Add Modal and upload object Todo: Make it all look nicer

`

huang0h

Awesome work guys!! A couple comments/questions about style and features:

huang0h · 2023-10-26T01:57:01Z

src/components/treePage/treeInfo.tsx

+          {userOwnsTree && treePresent && (
+            <TreePageUploadSiteImageButton siteData={siteData} />
+          )}


⛏️ I think we decided to allow anybody to upload images for a site, so we don't need to check if userOwnsTree here (sorry if the API doc said otherwise, I really need to clean that up 😞)

huang0h · 2023-10-26T02:00:21Z

src/components/treePage/treeInfo.tsx

+interface TreeUploadProps {
+  readonly siteData: SiteProps;
+}
+
+const TreePageUploadSiteImageButton: React.FC<TreeUploadProps> = ({
+  siteData,
+}) => {
+  return <UploadSiteImageButton siteEntryId={siteData.entries[0].id} />;
+};


❓ Just curious, how come we have this extra component rather than directly using UploadSiteImageButton? Was there some weird bug that didn't show the button without this?

huang0h · 2023-10-26T02:19:56Z

src/components/uploadSiteImageButton/index.tsx

+
+  const props: UploadProps = {
+    name: 'file',
+    multiple: false,


💭 Do we want to allow users to upload multiple images at a time? It'd be nice quality of life feature for users, but it'd take a bit more work to upload each image and handle individual errors. If not, I think we should add the maxCount prop as 1 (more info in the component doc) so it's clear to users that they can only upload one image at time.

huang0h · 2023-10-26T02:29:18Z

src/components/uploadSiteImageButton/index.tsx

+      <Modal
+        title={t('uploadSiteImage.upload_title')} // pass on as input
+        visible={showMenu}
+        footer={null}
+        onCancel={() => setShowMenu(false)}
+        closeIcon={<StyledClose />}
+      >


🔧 Can we add a way for users to choose to upload their images anonymously? This would make it so when we view their images, it'll hide their username (as opposed to displaying it publicly). Something like a switch or a similar input would work well - make sure to update the API route to use this value as well!

huang0h · 2023-10-26T02:43:08Z

src/components/uploadSiteImageButton/index.tsx

+  return (
+    <>
+      <GreenButton
+        type="text"


⛏️ Just a quick style nitpick - can we change this button's type to primary? This will keep the upload button's appearance consistent with the adopt button when we hover over it (white text on light green, vs. black text on white like it is now)

huang0h · 2023-10-26T02:45:50Z

src/api/test/protectedApiClient.test.ts

@@ -2186,4 +2186,61 @@ describe('Admin Protected Client Routes', () => {
      expect(result).toEqual(response);
    });
  });
+
+  describe('uploadImage', () => {


huang0h

Looks awesome! Just a couple comments:

huang0h · 2023-11-12T19:54:19Z

src/components/uploadSiteImageButton/index.tsx


  const props: UploadProps = {
    name: 'file',
-    multiple: false,
+    multiple: true,
    beforeUpload: async (file) => {


❓ Just curious, did making these functions async make a difference on your end? When I got rid of the async/await, it didn't seem to make much of a difference, and looking at the upload props, it doesn't look like they need to be async functions - we should probably avoid async stuff if we can since it adds more complexity.

huang0h · 2023-11-12T19:58:48Z

src/components/uploadSiteImageButton/index.tsx

-        message.success('Sent!');
-        setShowMenu(!showMenu);
+    if (imageToUpload.length > 0) {
+      imageToUpload.forEach((image) => {


⛏️ Can we use Promise.allSettled (mdn reference) to handle all of these requests? This way, we can wait until all of them finish and handle all the results together with something like this:

const requests: Promise<void>[] = []; imageToUpload.forEach((image) => { if (image) { requests.push( protectedApiClient.uploadImage(siteEntryId, image, anonymousUpload), ); } }); Promise.allSettled(requests) .then((results) => { // results contains a list of resolved promises }) .finally(() => dispatch(getSiteData(id)));

huang0h · 2023-11-12T20:00:17Z

src/components/uploadSiteImageButton/index.tsx

@@ -102,6 +118,13 @@ const UploadSiteImageButton: React.FC<UploadImageProps> = ({ siteEntryId }) => {
        <ConfirmUpload onClick={onClickUploadSiteImage}>
          {t('uploadSiteImage.upload_button_message')}
        </ConfirmUpload>
+        <Checkbox
+          onChange={async (e: CheckboxChangeEvent) => {


❓ Similar to the beforeChange async function above - this seemed to work without being async, did it make a difference for you?

TODO: Fix remove image

huang0h

lgtm! I wasn't able to figure out a way to get selecting multiple files in the file dialog to work (but we can still upload multiple files, if you select them one-by-one). I think we should log this issue for next semester and take another look with fresh eyes, but you're absolutely welcome to look into it too if you'd like!

huang0h · 2023-12-04T00:45:27Z

src/components/uploadSiteImageButton/index.tsx

  const [anonymousUpload, setAnonymousUpload] = useState(false);
  const dispatch = useDispatch();
  const id = Number(useParams<TreeParams>().id);

  const props: UploadProps = {
    name: 'file',
    multiple: true,
-    beforeUpload: async (file) => {
+    beforeUpload: (_, fileList) => {
+      console.log('before upload');


⛏️ Make sure to remove any console.log() calls before you merge! It's a tiny optimization for production, but we'd still like to get rid of them if we can (also our linter will throw a fit if it finds any console.log() calls 🤷)

…r-the-trees-frontend into SK.CL.upload-site-image

…dded

CiciLing and others added 17 commits September 28, 2023 21:00

cici update

8e8664a

Upload-Site-Images:

f48c868

- Add basic text - Add component definition - Add Modal and upload object Todo: Make it all look nicer

Update Button Formatting, fix prettier errors on client

e6d2887

api client test

c84d561

created test for api client

a8a7a70

formatting, api call

4b273bf

Add beforeUpload field

4d7d806

Add Site ID as non hardcoded value

7fb2dbf

Prettier

4848ff2

`

Prettier Testing

e0d6fa0

Fix upload route, add anonymous field, add base64 encoding

00221a4

Replace site id with id of first site entry

8544e7f

Cleanup

0725e43

site entry id

94f36db

Fix failing tests and dragger import

ff61cc6

Add constants of strings to treeInfo.json

d821212

Close window on upload

6c5870d

CiciLing requested review from Tvcz, huang0h, nalinikantheti and suubimagoola October 22, 2023 19:57

huang0h reviewed Oct 26, 2023

View reviewed changes

SurabhiKeesara added 4 commits November 1, 2023 18:43

Begin addressing review comments

9b32bbd

Address Comments

8516c61

Add Anonymous Uploads

92ad6bc

Cleanup

26d744a

SurabhiKeesara requested a review from huang0h November 12, 2023 18:24

huang0h reviewed Nov 12, 2023

View reviewed changes

SurabhiKeesara added 2 commits December 3, 2023 13:57

Address review comments

3e2c9cc

TODO: Fix remove image

Fix remove image

213a763

Fix async

9b62fb8

SurabhiKeesara requested a review from huang0h December 3, 2023 19:28

huang0h reviewed Dec 4, 2023

View reviewed changes

SurabhiKeesara added 4 commits April 14, 2024 18:26

Fix multiple image loading

337a412

Merge branch 'master' of https://github.com/Code-4-Community/speak-fo…

730435f

…r-the-trees-frontend into SK.CL.upload-site-image

fix syntax error

2434898

Fix syntax

cbdf5e0

SurabhiKeesara requested a review from huang0h April 17, 2024 14:52

huang0h previously approved these changes Apr 17, 2024

View reviewed changes

Fix bug with not allowing images from multiple selections to all be a…

37cf810

…dded

SurabhiKeesara dismissed huang0h’s stale review via 37cf810 April 18, 2024 06:10

huang0h merged commit eb3feea into master Apr 18, 2024
4 of 5 checks passed

huang0h deleted the SK.CL.upload-site-image branch April 18, 2024 21:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sk.cl.upload site image #295

Sk.cl.upload site image #295

CiciLing commented Oct 22, 2023 •

edited by SurabhiKeesara

Loading

huang0h left a comment

huang0h Oct 26, 2023

huang0h Oct 26, 2023

huang0h Oct 26, 2023

huang0h Oct 26, 2023

huang0h Oct 26, 2023

huang0h Oct 26, 2023

huang0h left a comment

huang0h Nov 12, 2023

huang0h Nov 12, 2023

huang0h Nov 12, 2023

huang0h left a comment

huang0h Dec 4, 2023

Sk.cl.upload site image #295

Sk.cl.upload site image #295

Conversation

CiciLing commented Oct 22, 2023 • edited by SurabhiKeesara Loading

Checklist

Why

This PR

Screenshots

Screen recording

Verification Steps

huang0h left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huang0h left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

huang0h left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CiciLing commented Oct 22, 2023 •

edited by SurabhiKeesara

Loading