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

Enhancements/Fixes #35

Open
emilyparkes opened this issue Sep 25, 2022 · 0 comments
Open

Enhancements/Fixes #35

emilyparkes opened this issue Sep 25, 2022 · 0 comments

Comments

@emilyparkes
Copy link

emilyparkes commented Sep 25, 2022

localhost:3000

  • There should be no errors on the console at all in your main working branch!
  • I shouldn't be able to see a delete button if i'm not meant to be able to? AM I meant to be able to update a story too?

Client-Side

actions

  • All promises need to have error handling. Add a .catch() to all thunk actions.

apis

  • In the stories file, you have return res.body but no promise to get res from!
  • This is duplicate code, move it to a utils file and then import where needed!
function logError(err) {
  if (err.response.text === 'Username Taken') {
    throw new Error('Username already taken - please choose another')
  } else if (err.message === 'Forbidden') {
    throw new Error(
      'Only the user who added the story may update and delete it'
    )
  } else {
    // eslint-disable-next-line no-console
    console.error('Error consuming the API (in client/api.js):', err.message)
    throw err
  }
}

components

  • There are multiple empty lines in your components. Use a linter and at one have one empty line to break up code max.
  • If you have Te Reo for some of your options, why not all?
  • You have no value for some of your inputs. They should have values.
  • Home.tsx, should use react-router-dom's Link component instead of <a href=''/>.
  • This should be a button, it is doing an action (the onClick) rather than just linking, have the onClick function navigate after completing the action behaviour. Links navigate you to different pages that is not the default behaviour of interacting with this text. Users expect an action.
<Link className="navLink" to="/" onClick={handleLogOff}>
            Log off
          </Link>
  • There is a lot of commented out code and unused funcs in this component. Please tidy up for in use code only.
  • You have broken code in Story.tsx:
<button onClick={() => navigate('/stories/{story.id}/update')}>
            Update Story
 </button>
  • Do you sometime lose one region? Meaning you need a check on each individual region?
 <option value={allRegions[4]?.id}>Auckland</option>
                  <option value={allRegions[5]?.id}>Waikato</option>
                  <option value={allRegions[6]?.id}>Bay of Plenty</option>
                  <option value={allRegions[7]?.id}>Gisborne</option>

Perhaps you could just check the array exists/has a length before rendering options instead?

Errors

  • Variable/property errors in Map.tsx, actions/regions.js, and reducers/search.js.

Server-Side

db

  • There is a file fruits.js that has stories stuff in it? Should this be deleted? Renamed?
  • Remove unused/commented out code

Secrets should be in an .env file! (and not pushed up to GitHub!)

  • You've exposed your secret key so you'll need to get a new one and make sure it's not exposed!
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

No branches or pull requests

1 participant