Skip to content

Palautteet ja niiden käsittely | Antti/feedback-after-logout#33

Merged
anttiasmala merged 114 commits intomainfrom
antti/feedback-after-logout
Oct 9, 2024
Merged

Palautteet ja niiden käsittely | Antti/feedback-after-logout#33
anttiasmala merged 114 commits intomainfrom
antti/feedback-after-logout

Conversation

@anttiasmala
Copy link
Copy Markdown
Collaborator

Tässä on ensimmäinen versio palautteiden käsittelystä 😄

…imer to redirect user to mainpage if they have not started to write anything
…quest as well, but it is not in use yet. Returns not in use text
@vercel
Copy link
Copy Markdown

vercel Bot commented Sep 3, 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 Oct 9, 2024 5:21pm

Comment thread TODO.md
Comment thread package.json Outdated
Comment thread backend/feedback.ts Outdated
Comment thread backend/feedback.ts Outdated
Comment thread backend/feedback.ts Outdated
Comment thread pages/api/auth/logout.ts
Comment thread pages/api/feedback/index.ts
@anttiasmala
Copy link
Copy Markdown
Collaborator Author

Nyt pitäisi olla kaikkiin kommentteihisi vastattu. Lisäsin myös muutaman kommentin

Onko ok, jos tulevaisuudessa painan Resolve conversation, kun kyseinen kommentti on valmistunut? Helpottaa sun kommenttien löytämistä Conversations-kohdasta. Löysin Conversationsin Files Changed -osion alta aivan sattumalta, näyttäisi ainakin olevan ihan näppärä. Conversationsin kautta löysin jonkun sun kommentin mitä ei ollut tullut vastaan missään muualla 😄

image

@samuliasmala
Copy link
Copy Markdown
Owner

Onko ok, jos tulevaisuudessa painan Resolve conversation, kun kyseinen kommentti on valmistunut? Helpottaa sun kommenttien löytämistä Conversations-kohdasta.

Tehdään mielummin niin, että mä klikkaan sen sitten kun oon varmistanut, että kyseinen kommentti on tehty onnistuneesti.

Löysin Conversationsin Files Changed -osion alta aivan sattumalta, näyttäisi ainakin olevan ihan näppärä. Conversationsin kautta löysin jonkun sun kommentin mitä ei ollut tullut vastaan missään muualla 😄

Tämähän oli hyvä, en ole itsekään huomannut tuommoista ominaisuutta 😆

@anttiasmala
Copy link
Copy Markdown
Collaborator Author

Koodissa on vielä melko paljon "CHECK THIS" -kommentteja, mutta poistan ne ennen mergeä. Niiden tarkoitus on muistuttaa itseäni mitkä kohdat on hyvä kommentoida / minkä takia tein jonkun tietyn asian tietyllä tavalla.

Teen monesti useampaa branchia samaan aikaan, joten ei ilman noita muista miksi tein kun tein

Copy link
Copy Markdown
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.

Muutama avoin kohta vielä.

return {
props: {
user: JSON.parse(JSON.stringify(cookieData.user)) as User,
user: JSON.parse(JSON.stringify(returnThis.data)) as User,
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Miksi JSON.parse(JSON.stringify()) peräkkäin? Jos tarkoitus kloonata objekti, niin structuredClone on parempi.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Syy JSON.parse(JSON.stringify()):n käyttöön on se, että structruredClone:lla heittää seuraavan virheen: Uncaught Error: Error serializing '.user.createdAt' returned from 'getServerSideProps' in "/logout". Reason: 'object' ("[object Date]") cannot be serialized as JSON. Please only return JSON serializable data types.

https://stackoverflow.com/a/72837265

Tuossa joku oli sanonut, että instead, you can use JSON.stringify inside JSON.parse to create a serializable object

En ainakaan keksinyt mitään helppoa reittiä structuredClonen käyttämiseen 😄

Copy link
Copy Markdown
Owner

@samuliasmala samuliasmala Oct 6, 2024

Choose a reason for hiding this comment

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

Nyt tulee mieleen että olen tainnut joskus aikaisemminkin kysyä tätä 😆 Lisäsin yhden kommentin, missä selitin miksi käytetään JSON.stringify ja JSON.parse niin en kysy enää toista kertaa.

Comment thread TODO.md
Comment thread pages/api/auth/logout.ts Outdated
Comment thread pages/logout.tsx Outdated
Comment thread pages/api/feedback/index.ts Outdated
Comment thread backend/auth.ts
Comment on lines +116 to +132

/**
* Checks for a valid user and session, **REGARDLESS** of login status
*
* **ATTENTION**: the user **DOES NOT** need to be logged in for this function to validate request
*/

export async function checkIfSessionValid(
req: NextApiRequest,
res: NextApiResponse,
): Promise<{ user: User; session: LuciaSession }> {
const userData = await validateRequest(req, res);
if (!userData.session || !userData.user) {
throw new HttpError('You are unauthorized!', 401);
}
return userData;
}
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Tämä on siirretty muiden "tarkastajien" luokse. Lisäsin pienen JSDoc-muistutuksen itselle, koska oon joutunut monesti tsekkaamaan, että mitä tuo funktio tekikään

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Tuollaiset JSDoc-dokumentaatiot ovat erinomainen tapa 👍

Comment thread pages/index.tsx Outdated
@anttiasmala
Copy link
Copy Markdown
Collaborator Author

Nyt pitäisi olla kaikkiin vastattu 😄 👍

Copy link
Copy Markdown
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.

Valmis mergettäväksi 💪

@anttiasmala anttiasmala merged commit 616c0cb into main Oct 9, 2024
@anttiasmala anttiasmala deleted the antti/feedback-after-logout branch October 9, 2024 17: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.

9. Backend-tuki palautteille

2 participants