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

feat: clear image based on preset #2488

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

feat: clear image based on preset #2488

wants to merge 3 commits into from

Conversation

sshanzel
Copy link
Member

Rather than having a nullable prop that would signify that we should clear the image on certain mutations, we introduced a mutation to specifically do it. This makes maintaining how the clearance works much easier than remembering the meaning of the passed null values.

Copy link
Contributor

@capJavert capJavert left a comment

Choose a reason for hiding this comment

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

Nice solution some non blocking comments/ideas.

@@ -1770,6 +1779,31 @@ export const resolvers: IResolvers<unknown, BaseContext> = traceResolvers<
},
},
Mutation: {
clearImage: async (
_,
{ preset }: { preset: UploadPreset },
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe preset can be an array so we can remove in a single go as well? Not blocking.

@@ -49,10 +49,10 @@ export class User {
email: string;

@Column({ type: 'text', nullable: true })
image: string;
image?: string | null;
Copy link
Contributor

Choose a reason for hiding this comment

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

I hate typeorm

Comment on lines +1789 to +1791
await con.getRepository(User).update(userId, {
cover: null,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
await con.getRepository(User).update(userId, {
cover: null,
});
await con.getRepository(User).update(
{
id: userId,
},
{
cover: null,
},
);

just to be more safe that we are targeting id column, also adjust below

@@ -104,6 +104,16 @@ type PostPreset =
| UploadPreset.FreeformImage
| UploadPreset.FreeformGif;

export const clearFile = (referenceId: string, preset: UploadPreset) => {
if (!process.env.CLOUDINARY_URL) {
return Promise.resolve();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return Promise.resolve();
return

@@ -104,6 +104,16 @@ type PostPreset =
| UploadPreset.FreeformImage
| UploadPreset.FreeformGif;

export const clearFile = (referenceId: string, preset: UploadPreset) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

use object syntax { referenceId, preset }, easier to add new params later?

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