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

Minimum viable product (Closes #11) #20

Merged
merged 31 commits into from
Apr 7, 2025
Merged

Minimum viable product (Closes #11) #20

merged 31 commits into from
Apr 7, 2025

Conversation

memagu
Copy link
Member

@memagu memagu commented Apr 6, 2025

Minimum viable (barely) product finished. Issues on what needs to be done further will be created shortly.

Future work includes:

  • styling
  • autopopulate form fields
  • reset image on successful theme creation
  • non shared form feedback
  • queue status
  • atomize db-interactions
  • improve err messages
  • set timeout for auth-cookies
  • full typing
  • fix linting and cheking

memagu added 24 commits March 24, 2025 21:44
Actually not so minor. SSE now works, REAL TIME updates achieved (race condition here we come). Also there is lots of cleaning needing to be done. PLease contanct Simon Mechler for tidying services :D
TODO: Fix full/partial schemas, add/remove pubs/themes/keys, secure sse, add queue status (maybe), styling
Some big issues needs to be fixed before the MVP is complete. The smaller ones and styling will be left for further development
Improve development guide
@memagu memagu added this to CPU Apr 6, 2025
@github-project-automation github-project-automation bot moved this to 🆕 New in CPU Apr 6, 2025
@memagu memagu added the enhancement New feature or request label Apr 6, 2025
@memagu memagu changed the title Minimum viable product Minimum viable product (Closes #11) Apr 7, 2025
Copy link
Contributor

@Fiery-132 Fiery-132 left a comment

Choose a reason for hiding this comment

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

Overall, I think it looks very good! 😄

I added a few comments and suggestions (some are quite nitpicky, sorry.. 😅). So just address them and go ahead and merge. 👍

Comment on lines 26 to 28
if (cookies.get("adminKey") !== ADMIN_KEY) {
return fail(401, { message: "Unauthorized" });
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Authorisation is performed on every action so maybe extract that to a separate function?

Comment on lines +30 to +45
const formData = Object.fromEntries(await request.formData());

const result = pubKeyIdPairSchema.pick({
pubKey: true,
pubId: true,
})
.safeParse(formData);

if (!result.success) {
const { fieldErrors } = result.error.flatten();

return fail(400, {
errors: fieldErrors,
values: result.data,
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This sequence is also very repeated:

const formData = ...
const result = ...
if (!result.success) { ... }

Perhaps make some kind of helper function like:

async function parseForm<T>(request: Request, schema: ZodSchema<T>) {
  const formData = Object.fromEntries(await request.formData());
  const result = schema.safeParse(formData);
  if (!result.success) {
    const { fieldErrors } = result.error.flatten();
    return fail(400, { errors: fieldErrors, values: result.data });
  }
  return result.data;
}

Comment on lines +27 to +32
const pubKey: PubKey = cookies.get("pubKey");
const pubKeyIdPairs: PubKeyIdPairs = await getPubKeyIdPairs();

if (!pubKey || !pubKeyIdPairs.has(pubKey)) {
return fail(401, { message: "Unauthorized" });
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I would probably extract this to avoid repetition. 😄

memagu and others added 7 commits April 7, 2025 17:45
DEtta är bra

Co-authored-by: Simon Mechler <[email protected]>
Sounds good to me

Co-authored-by: Simon Mechler <[email protected]>
This is on you if it fails >:)

Co-authored-by: Simon Mechler <[email protected]>
Sounds good, will implement before merging

Co-authored-by: Simon Mechler <[email protected]>
@memagu memagu merged commit 42d09a0 into main Apr 7, 2025
1 check passed
@github-project-automation github-project-automation bot moved this from 🆕 New to ✅ Done in CPU Apr 7, 2025
@memagu memagu deleted the minimum-viable-product branch April 7, 2025 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

2 participants