created link button to new item #227
Conversation
👷 Deploy request for cool-creponne-3e1272 pending review.Visit the deploys page to approve it
|
Added new button, 'GO TO YOUR ITEM' |
aaleksa
force-pushed
the
#192-Add-a-link-to-a-newly-created-item-on-add-item-success-page
branch
2 times, most recently
from
70cd473
to
2a2ade4
Compare
There was a problem hiding this comment.
Thanks @aaleksa - your implementation is working brilliantly 👍 but I'm requesting a few changes that should help us clean up the code base a little.
It's also just worth adding a reminder to please revisit our documentation on conventions - the conventional commit messages for example won't be a problem if they're missed but in a more complicated PR (and especially if we ever need to go back in the commit history and track an issue) it can help us greatly keeping track of what changes are happening throughout our commit history.
| @@ -10,6 +17,9 @@ const SuccessPage = () => { | |||
| <Link href='/add-item'> | |||
| <ButtonRounded type='button'>ADD ANOTHER ITEM</ButtonRounded> | |||
| </Link> | |||
| <Link href={`/item/${idItem}`}> | |||
| <ButtonGoToItem type='button'>GO TO YOUR ITEM</ButtonGoToItem> | |||
There was a problem hiding this comment.
We don't need a new component here. The ButtonRounded.tsx component can be re-used for this situation by just passing the appropriate props - let's apply it instead 👍
| const response = await supabase.from(table).insert(InsertValues).select(); | ||
| const [createdRecord] = response?.data ?? []; | ||
| return createdRecord as PartialItem; | ||
| } catch (error) { | ||
| // setSubmitting(false); | ||
| console.log('An unexpected error occurred'); |
There was a problem hiding this comment.
| const response = await supabase.from(table).insert(InsertValues).select(); | |
| const [createdRecord] = response?.data ?? []; | |
| return createdRecord as PartialItem; | |
| } catch (error) { | |
| // setSubmitting(false); | |
| console.log('An unexpected error occurred'); | |
| const {data, error} = await supabase.from(table).insert(InsertValues).select(); | |
| return data as PartialIem; | |
| } catch (error) { | |
| throw error | |
| console.error(`Error inserting rows to the database: ${error}`); |
Using the nullish coalescing operator here will mean we always get an array even if there was an error - we definitely don't want that because it would hide any issues happening in our query to the database.
Instead, we can refactor the function to actually throw the error back up to be handled appropriately wherever we called this query.
| const response = await insertRow('items', dataItem); | ||
| if (response === false) { | ||
| throw 'InsertFailed'; |
There was a problem hiding this comment.
Based on the refactor for insertRow suggested in this review let's also adapt this implementation of the function so we can handle the error it would throw in case of a failure. It should just be a case of replicating the same pattern.
💡 Also, I really liked the fact that you attempted to enforce our types in your refactor of insertRow but let's move that declaration up to here instead.
I'm suggesting this because I think it will be a better separation of concerns - it means we let our util return the data just as is and then we get TypeScript to evaluate if it matches the type we expected once it's been returned here. That way the static testing will be working for us and alerting us if anything changes in future and our Types need a review or there is an issue with the data returned.
| .button-stroke { | ||
| border-left: #54BB89 solid 8px; | ||
| border-right: #54BB89 solid 8px; | ||
| } |
There was a problem hiding this comment.
I'm a little confused about the intention of this new class - I couldn't really make out the effect from looking at the UI, can you share what this is fixing in the button?
|
Relates #192 Just explicitly linking to Issue for traceability |
|
Hi @aaleksa can you just confirm that you are still monitoring this PR? |
|
Closing this PR as it's been stale and the issue is up for re-assignment |
Checklist:
Description
Relates #[issue number] OR Closes #[issue number]
Include here a short description of what behaviour this Pull Request introduces.
Outline any new dependencies introduced.
Files changed
Include one to two lines on the necessary changes to each file.
Added itemId to the path in router redirect
Added Button
ButtonGoToItem implementation
Return created item record on api call "supabase.from(table).insert(InsertValues).select();" to create new item
UI changes
If this PR includes changes to the UI include a screengrab of the alterations in review.
Changes to Documentation
Include an outline of why Documentation needed to be updated with this PR
Tests
Include a description of any tests added - if none have been added explain why.