Code review

[bas-ie]

Don't merge this: it's just so we can write comments on your code so far! We'll tell you when they're ready to view.

[bas-ie]

There's some awesome work going on in here! And I know there's a bunch in branches too that hasn't made it into development. Keep up the hard work!

A general comment: if you can agree on the names of things, it really helps keep the code straight in your head. So for example, a record might always be a record and never a date, or an activity might be an activity but not a card. It seems like a minor thing, but having consistent names is really useful 👍

[bas-ie]

It's a little weird that this is getRecordPending and not addRecordPending. Try to be really specific about naming, it'll help you in the long run when you have fifty or sixty of these things!

Remember, if you find that you're doing the same thing with each action in the reducers, you can just list all the action types above one another:

switch (action.type) {
  case GET_RECORD_PENDING:
  case ADD_RECORD_PENDING:
  case DEL_RECORD_PENDING:
    return {
      pending: true
    }
}

[bas-ie]

Not quite sure why the <a> here... is it just to get the mouse pointer? You can do that in CSS with cursor: pointer;.

[bas-ie]

I think we've already fixed this, but just be aware that if you pass mapDispatchToProps as the first function to connect, it'll be treated as mapStateToProps (which will break). Instead, pass a null:

export default connect(null, mapDispatchToProps)(ActivityCard)

[bas-ie]

A nice shortcut if you only want a few properties out of the store is this:

const mapStateToProps = ({ activities, user }) => ({ activities, user })

[bas-ie]

You can also use:

<Loader active />

see docs: https://react.semantic-ui.com/elements/loader/#states-active

[bas-ie]

As you've probably already discovered, this won't work with SQLite3 (although it'll be fine with Postgres).

[bas-ie]

The .returning, that is 😄

[bas-ie]

It's a good idea to specify which HTTP status code you want to send back to the client, especially for errors. Without it, there's a risk that Superagent will think everything's fine, and won't reject the promise... so your error will not end up in the .catch block (in the async action creator). So something like:

  .catch(err => res.status(500).json({ ok: false, error: err.message })

[bas-ie]

You could also use something like:

const parsedRecords = records.map(r => {
  // rec is now everything _except_ activityId
  const { activityId, ...rec } = r

  return {
    date_id: dateId[0],
    activity_id: activityId,
    ...rec
  })

There are other approaches to converting camel to snake case, including lodash.

[bas-ie]

Probably use const for most things unless it's actually generating an error. It can protect you against subtle bugs, where you reassign something you didn't mean to.

[bas-ie]

There's quite a lot of JavaScript processing the data in this function. I'm almost positive the same thing can be accomplished with a join (using Knex). Wherever possible, try to get the database to do this stuff for you as it's much, much faster than JS will ever be 😄