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

Fix TODO for setting up openapi-fetch #214

Merged
merged 9 commits into from
Jul 31, 2024
Merged

Fix TODO for setting up openapi-fetch #214

merged 9 commits into from
Jul 31, 2024

Conversation

antonyr
Copy link
Contributor

@antonyr antonyr commented Jul 31, 2024

This fixes @codekansas comments TODO.

however listing_id is hardcoded. I am not sure if we are going to use ImageUpload /UDRFUpload components.

@antonyr antonyr requested a review from codekansas July 31, 2024 13:41
Comment on lines 9 to 18
const authMiddleware: Middleware = {
async onRequest({ request }) {
const accessToken = localStorage.getItem("AUTH");
if (!accessToken) {
throw new Error("No access token found");
}

request.headers.set("Authorization", `Bearer ${accessToken}`);
return request;
},
Copy link
Member

Choose a reason for hiding this comment

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

ah interesting, i didn't realize this code path still existed, seems like we should merge with auth.tsx

Comment on lines 17 to 18
fd.append("file", file);
fd.append("metadata", JSON.stringify(request));
Copy link
Member

Choose a reason for hiding this comment

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

looks good

// console.error("Error uploading file:", error);
// }
const { error } = await APICalls.upload(selectedFile, {
artifact_type: "image",
Copy link
Member

Choose a reason for hiding this comment

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

should be urdf here?

// });
const { response, error } = await APICalls.upload(compressedFile, {
artifact_type: "image",
listing_id: "5",
Copy link
Member

Choose a reason for hiding this comment

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

should add listing_id to props i believe? like the component should be used to upload an image for an associated listing

Comment on lines 47 to 57
const authMiddleware: Middleware = {
async onRequest({ request }) {
const accessToken = localStorage.getItem("AUTH");
if (!accessToken) {
throw new Error("No access token found");
}

request.headers.set("Authorization", `Bearer ${accessToken}`);
return request;
},
};
Copy link
Member

Choose a reason for hiding this comment

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

this is duplicated below

Comment on lines 50 to 52
if (!accessToken) {
throw new Error("No access token found");
}
Copy link
Member

Choose a reason for hiding this comment

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

this is incorrect, no? what if the user is not logged in - they should still be able to make api calls to the public endpoint?

@codekansas codekansas merged commit e027be0 into master Jul 31, 2024
1 check passed
@codekansas codekansas deleted the fix-openapi-fetch branch July 31, 2024 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants