Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sk.cl.upload site image #295

Merged
merged 29 commits into from
Apr 18, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions src/api/protectedApiClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,10 @@ export interface ProtectedApiClient {
readonly filterSites: (
params: FilterSitesParams,
) => Promise<FilterSitesResponse>;
readonly uploadImage: (
siteEntryId: number,
imageFile: string | ArrayBuffer,
) => Promise<void>;
}

export enum ProtectedApiClientRoutes {
Expand Down Expand Up @@ -197,6 +201,8 @@ export const ParameterizedApiRoutes = {
UPDATE_SITE: (siteId: number): string => `${baseSiteRoute}${siteId}/update`,
NAME_SITE_ENTRY: (siteId: number): string =>
`${baseSiteRoute}${siteId}/name_entry`,
UPLOAD_IMAGE: (siteEntryId: number): string =>
`${baseSiteRoute}site_image/${siteEntryId}`,
};

export const ParameterizedAdminApiRoutes = {
Expand Down Expand Up @@ -564,6 +570,16 @@ const filterSites = (
).then((res) => res.data);
};

const uploadImage = (
siteEntryId: number,
imageFile: string | ArrayBuffer,
): Promise<void> => {
return AppAxiosInstance.post(
ParameterizedApiRoutes.UPLOAD_IMAGE(siteEntryId),
{ anonymous: false, image: imageFile },
).then((res) => res.data);
};

const Client: ProtectedApiClient = Object.freeze({
makeReservation,
completeReservation,
Expand Down Expand Up @@ -612,6 +628,7 @@ const Client: ProtectedApiClient = Object.freeze({
addSites,
sendEmail,
filterSites,
uploadImage,
});

export default Client;
57 changes: 57 additions & 0 deletions src/api/test/protectedApiClient.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2186,4 +2186,61 @@ describe('Admin Protected Client Routes', () => {
expect(result).toEqual(response);
});
});

describe('uploadImage', () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

let imageToUpload: string | ArrayBuffer;

beforeEach(() => {
const reader = new FileReader();
const imageFile = new File(['image-data'], 'image.jpg', {
type: 'image/jpeg',
});
reader.addEventListener(
'loadend',
() => {
imageToUpload = reader.result ?? '';
},
false,
);
reader.readAsDataURL(imageFile);
});
it('makes the right request', async () => {
const response = 'Image uploaded successfully';
nock(BASE_URL)
.post(ParameterizedApiRoutes.UPLOAD_IMAGE(11934))
.reply(200, response);

const result = await ProtectedApiClient.uploadImage(11934, imageToUpload);

expect(result).toEqual(response);
});

it('makes a bad request', async () => {
const response = 'invalid site entry id';

nock(BASE_URL)
.post(ParameterizedApiRoutes.UPLOAD_IMAGE(-1))
.reply(400, response);

const result = await ProtectedApiClient.uploadImage(
-1,
imageToUpload,
).catch((err) => err.response.data);

expect(result).toEqual(response);
});

it('invalid or incorrectly formatted image file', async () => {
const response = 'Invalid image format';

nock(BASE_URL)
.post(ParameterizedApiRoutes.UPLOAD_IMAGE(11934))
.reply(400, response);

const result = await ProtectedApiClient.uploadImage(
11934,
imageToUpload,
).catch((err) => err.response.data);
});
});
});
15 changes: 15 additions & 0 deletions src/components/treePage/treeInfo.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import { site } from '../../constants';
import { n } from '../../utils/stringFormat';
import { isSFTT } from '../../utils/isCheck';
import { getCommonName } from '../../utils/treeFunctions';
import UploadSiteImageButton from '../uploadSiteImageButton';

const TreeHeader = styled.div`
text-transform: capitalize;
Expand Down Expand Up @@ -163,6 +164,10 @@ export const TreeInfo: React.FC<TreeProps> = ({
isAdopted={isAdopted}
/>

{userOwnsTree && treePresent && (
<TreePageUploadSiteImageButton siteData={siteData} />
)}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛏️ 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 😞)


{userOwnsTree && treePresent && (
<StewardshipContainer>
<Typography.Title level={3}>
Expand Down Expand Up @@ -235,3 +240,13 @@ const TreePageShareButton: React.FC<TreePageShareButtonProps> = ({
/>
);
};

interface TreeUploadProps {
readonly siteData: SiteProps;
}

const TreePageUploadSiteImageButton: React.FC<TreeUploadProps> = ({
siteData,
}) => {
return <UploadSiteImageButton siteEntryId={siteData.entries[0].id} />;
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ 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?

110 changes: 110 additions & 0 deletions src/components/uploadSiteImageButton/index.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
import React, { useState } from 'react';
import styled from 'styled-components';
import { Modal } from 'antd';
import { useTranslation } from 'react-i18next';
import { site } from '../../constants';
import { n } from '../../utils/stringFormat';
import { GreenButton, StyledClose, SubmitButton } from '../themedComponents';
import { InboxOutlined } from '@ant-design/icons';
import { message, Upload } from 'antd';
import { UploadProps } from 'antd/lib/upload/interface';
import { LIGHT_GREEN, LIGHT_GREY } from '../../utils/colors';
import protectedApiClient from '../../api/protectedApiClient';

const { Dragger } = Upload;

const StyledInboxOutline = styled(InboxOutlined)`
color: black;
`;

const ConfirmUpload = styled(SubmitButton)`
margin: 10px;
padding-left: 10px;

& :hover {
background-color: ${LIGHT_GREY};
}
`;

interface UploadImageProps {
readonly siteEntryId: number;
}

const UploadSiteImageButton: React.FC<UploadImageProps> = ({ siteEntryId }) => {
const { t } = useTranslation(n(site, ['treeInfo']), {
nsMode: 'fallback',
});
const [showMenu, setShowMenu] = useState(false);
let imageToUpload: string | ArrayBuffer | null;

const props: UploadProps = {
name: 'file',
multiple: false,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 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.

beforeUpload: async (file) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ 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.

const reader = new FileReader();
reader.addEventListener(
'loadend',
() => {
imageToUpload = reader.result;
},
false,
);
reader.readAsDataURL(file);
return false;
},
onChange(info) {
const { status } = info.file;
if (status === 'done') {
message.success(`${info.file.name} file selected successfully.`);
} else if (status === 'error') {
message.error(`${info.file.name} file upload failed.`);
}
},
};

function onClickUploadSiteImage() {
if (imageToUpload) {
protectedApiClient.uploadImage(siteEntryId, imageToUpload).then(() => {
message.success('Sent!');
setShowMenu(!showMenu);
});
}
}

return (
<>
<GreenButton
type="text"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛏️ 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)

onClick={() => {
setShowMenu(!showMenu);
}}
>
Upload Tree Images
</GreenButton>
<Modal
title={t('uploadSiteImage.upload_title')} // pass on as input
visible={showMenu}
footer={null}
onCancel={() => setShowMenu(false)}
closeIcon={<StyledClose />}
>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔧 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!

<Dragger {...props}>
<p className="ant-upload-drag-icon">
<StyledInboxOutline style={{ color: LIGHT_GREEN }} />
</p>
<p className="ant-upload-text">
{t('uploadSiteImage.upload_drag_header')}
</p>
<p className="ant-upload-hint">
{t('uploadSiteImage.upload_drag_description')}
</p>
</Dragger>
<ConfirmUpload onClick={onClickUploadSiteImage}>
{t('uploadSiteImage.upload_button_message')}
</ConfirmUpload>
</Modal>
</>
);
};

export default UploadSiteImageButton;
6 changes: 6 additions & 0 deletions src/i18n/en/treePage/treeInfo.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,12 @@
"adopt": "Adopt",
"force_unadopt": "Force Unadopt"
},
"uploadSiteImage": {
"upload_title": "Upload an Image of this Site",
"upload_drag_header": "Click or drag file to this area to upload",
"upload_drag_description": "Upload an image of your adopted tree!",
"upload_button_message": "Upload"
},
"log_in": "Log in to adopt this tree!",
"name_prompt": "Name this tree!"
}
Loading