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

DRAFT: Add API endpoints for POST requests #18

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

hojott
Copy link

@hojott hojott commented Sep 12, 2024

The endgoal would be to also have endpoints for modifying events, registering to events, ...

@hojott hojott self-assigned this Sep 12, 2024
@@ -89,6 +91,21 @@ async function startServer(servicePort: number) {
app.listen(servicePort, () =>
console.log('App listining on port', servicePort)
)

app.post(
'/api/events/add',
Copy link
Member

Choose a reason for hiding this comment

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

Pelkkä /api/events olisi enemmän REST-mäista.

authorizeRequest,
async (req, res) => {
try {
const field = await calendarEventService.createCalendarEvent(req.body)
Copy link
Member

Choose a reason for hiding this comment

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

Käyttäjältä saatu data (req.body) pitää kyllä jotenkin validoida, ennen kuin sitä päästää eteenpäin. Suosittelisin ottamaan projektiin mukaan Zod-kirjaston ja käyttämään sitä.

@@ -220,3 +220,57 @@ function parseUserEventsQueryResult(
): CalendarEvent & { price: string } {
return { ...parseQueryResult(row), price: row.price as string }
}

export async function createCalendarEvent(
props: {
Copy link
Member

Choose a reason for hiding this comment

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

Nimi props on vähän Reactismi. Olisko parempi nimi vaikka event.

@@ -220,3 +220,57 @@ function parseUserEventsQueryResult(
): CalendarEvent & { price: string } {
return { ...parseQueryResult(row), price: row.price as string }
}

export async function createCalendarEvent(
props: {
Copy link
Member

Choose a reason for hiding this comment

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

Tämä tyyppi olisi varmaan hyvä eriyttää omaksi tyyppimäärittelykseen (type NewEvent = { ... }). Jos otat käyttöön Zod-kirjaston, voi sitä myös hyödyntää tässä.

alcohol_meter: number | null,
}
): Promise<knex.Knex.QueryBuilder<any, number[]>> {
const calendarEvent = {
Copy link
Member

Choose a reason for hiding this comment

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

Jos props-muuttujan sisältö tarkastetaan jo pyynnön käsittelijässä, kuten on hyvän käytännön mukaista, ei tässä kohta enään tarvitse tehdä tällaista, vaan tiedot voi antaa suoraan eteenpäin Knexille.

Copy link
Author

Choose a reason for hiding this comment

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

tää on muute erinomaisen hyvä pointti :D

@dogamak
Copy link
Member

dogamak commented Sep 12, 2024

Tarvitaan varmaan myöskin erillinen API-avain kirjoitusoikeudelle, koska toi nykyinen read-only tokeni on mm. infonäytössä, jossa se on julkisena selaimen saatavilla.

@ConcernedHobbit ConcernedHobbit marked this pull request as draft September 23, 2024 18:01
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