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

Antti/api #10

Merged
merged 71 commits into from
Feb 15, 2024
Merged

Antti/api #10

merged 71 commits into from
Feb 15, 2024

Conversation

anttiasmala
Copy link
Collaborator

Keskeneräinen vielä. Tarkoituksena tehdä REST-apin tyylinen ratkaisu

…nctions for every request type and some basic test code
…jsonServerFunctions.ts to giftRequests.ts for now. Changed baseURL variable's string to use built api. Changed all request functions to work with the built api
…/catch blocks to all handles to get correct error messages. Might delete later if not needed
@anttiasmala anttiasmala linked an issue Feb 4, 2024 that may be closed by this pull request
Copy link

vercel bot commented Feb 4, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
lahjalista ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 14, 2024 10:28pm

@anttiasmala
Copy link
Collaborator Author

anttiasmala commented Feb 4, 2024

Pohjustuksena: rupesin tekemään https://nextjs.org/docs/pages/building-your-application/routing/api-routes tämän pohjalla, enkä löytänyt kaikkeen sopivia vastauksia, joten improvisoin. Nähtäväksi jää kuinka paljon menin metsään 😅

pages/api/gifts.ts Outdated Show resolved Hide resolved
pages/api/gifts.ts Outdated Show resolved Hide resolved
pages/api/gifts.ts Outdated Show resolved Hide resolved
pages/api/gifts.ts Outdated Show resolved Hide resolved
Copy link
Owner

@samuliasmala samuliasmala left a comment

Choose a reason for hiding this comment

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

Vastasin kysymyksiin. Lisäksi pushasin yhden commitin missä laitoin muutamia eslint-sääntöjä päälle jotka oli aiemmin otettu pois. Varoittavat anyn käytöstä mitä on yleensä syytä välttää. Sulla on jo sen verran kokemusta että pärjäät sääntöjen kanssa 🙂

pages/index.tsx Show resolved Hide resolved
pages/api/gifts.ts Outdated Show resolved Hide resolved
pages/api/gifts.ts Outdated Show resolved Hide resolved
pages/api/gifts.ts Outdated Show resolved Hide resolved
pages/api/gifts.ts Outdated Show resolved Hide resolved
…ed one check instead of every handler having their own check
…ted gift instead of given values of gift that is wanted to be created
…erty from newGift object. Added a variable for createGift function to get the gift data of created gift. Updated updatedGiftList variable to use correct data while concatting
Comment on lines 21 to 28
try {
const reqHandler = req.method !== undefined && HANDLERS[req.method];
if (reqHandler) {
if (typeof req.query.id !== 'string') {
throw new Error('Invalid ID', { cause: 'idError' });
}

await reqHandler(req, res);
Copy link
Collaborator Author

@anttiasmala anttiasmala Feb 13, 2024

Choose a reason for hiding this comment

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

Tein tämän, jotta jokaisessa Handler-funktiossa ei tarvitse alussa tarkistaa samaa asiaa. Tähän asiaan liittyen yksi kysymys alempana. Käytän tässä myös Errorissa cause:a. Oma Errori on tehty Postgres-versiossa, en sitä mergennyt tänne

Edit: typo korjattu

Comment on lines 25 to 30
if (typeof req.query.id !== 'string') {
throw new Error('Invalid ID', { cause: 'idError' });
}
queryID = req.query.id;

await reqHandler(req, res);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Tein tämän queryID-variablen, koska TypeScript ei tunnista tuota tarkistusta ylempänä. TypeScript ajattelee, että req.query.id voisi olla string | string[] | undefined, kun käsittelee handle-funktioita. Sen ei pitäisi kuitenkaan olla mahdollista, koska handle-funktiot kutusutaan vain tuon tarkistuksen jälkeen 😄

Olisiko tuohon jotain parempaa tapaa tehdä vai onko ihan ok? 😄

Copy link
Owner

@samuliasmala samuliasmala Feb 13, 2024

Choose a reason for hiding this comment

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

Idea on hyvä välttää toistoa, mutta ei toimi hyvin noin. Globaaleita muuttujia ei kannata käyttää, sillä nyt jos kaksi requestia tulee lähes yhtä aikaa ehtii jälkimmäinen request muuttaa queryID muuttujan arvon ennen kuin ensimmäinen ehtii käyttää sitä.

Tuo kannattaa tehdä niin, että muuttaa tuon yllä olevan rivin 28 näin: const queryId = req.query.id; Huomaat että sille tulee tyypiksi string, jonka voi sitten välittää funktion parametrina, eli noille HANDLER-funktioille pitää lisätä kolmas parametri.

Selkeyden vuoksi muuttaisin parametrit objektiksi, jolloin tulisi jotenkin näin

type HandlerParams = {
  req: NextApiRequest;
  res: NextApiResponse;
  queryId: string;
};

const HANDLERS: Record<string, (params: HandlerParams) => Promise<void>> = {
  GET: handleGET,
  PATCH: handlePATCH,
  PUT: handlePUT,
  DELETE: handleDELETE,
} as const;

async function handleGET({ req, res, queryId }: HandlerParams) {
  const giftRequest = await axios.get(`${baseURL}/${queryId}`);
  return res.status(giftRequest.status).send(giftRequest.data as Gift);
}

Ja itse asiassa handleGET funktiosta voi yltä poistaa req kun sitä ei tarvita.

utils/giftRequests.ts Outdated Show resolved Hide resolved
pages/api/gifts/[id].ts Outdated Show resolved Hide resolved
… added queryId as a new type. Fixed all the handles to have HandlerParams as type
…s custom message due to delete request doesn't seem to return anything by default
Comment on lines 60 to 65
async function handleDELETE({ res, queryId }: HandlerParams) {
const deleteRequest = await axios.delete(`${baseURL}/${queryId}`);
return res.status(deleteRequest.status).json(req.body);
return res
.status(deleteRequest.status)
.send('Lahja poistettu onnistuneesti!');
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Laitoin tuohon oman tekstin, kun lahja on poistettu, lienee melko turha.

Voisin laittaa kaikki virheet, return-statementistä lähetettävät tiedot yms englanniksi. Kääntää ne vain suomeksi sitten, kun tekee toastin.

Esim. että tuo 'Lahja poistettu onnistuneesti!' olisikin 'Gift deleted succesfully' yms

Copy link
Owner

Choose a reason for hiding this comment

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

Jep status-tekstit joita ei suoraan näytetä käyttäjälle kannattaa kirjoittaa englanniksi. Tähän en kuitenkaan laittaisi status-tekstiä, sillä yleensä DELETE-request ei palauta mitään. Se että lahjan poisto onnistui kerrotaan status-koodilla (200). Jos poisto ei jostain syystä onnistu palautetaan esim. koodi 400 tai 500.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Joo, näin ajattelinkin 👍

@anttiasmala
Copy link
Collaborator Author

Kaikki pitäisi olla korjattu. Pahoittelut jos ei ollutkaan. Halusin tehdä nämä vielä tänään vaikkakin myöhäiseksi menikin 😄

Copy link
Owner

@samuliasmala samuliasmala left a comment

Choose a reason for hiding this comment

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

Vielä pienet muutokset.

pages/api/gifts/[id].ts Outdated Show resolved Hide resolved
utils/giftRequests.ts Show resolved Hide resolved
pages/index.tsx Outdated Show resolved Hide resolved
Copy link
Owner

@samuliasmala samuliasmala left a comment

Choose a reason for hiding this comment

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

EDIT: vahingossa valitsin request changes.

Copy link
Owner

@samuliasmala samuliasmala left a comment

Choose a reason for hiding this comment

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

Erinomaista työtä! 🔥 Voi mergetä!

@anttiasmala anttiasmala merged commit ac148fb into main Feb 15, 2024
5 checks passed
@anttiasmala anttiasmala deleted the antti/api branch February 15, 2024 10:21
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.

3. Haetaan data json-serveriltä api:n kautta
2 participants