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

Live freelancer data #43

Merged
merged 9 commits into from
Mar 5, 2018
Merged

Live freelancer data #43

merged 9 commits into from
Mar 5, 2018

Conversation

Jskobos
Copy link
Collaborator

@Jskobos Jskobos commented Feb 8, 2018

Addresses #24 (minus the CSS refactoring)

- Data is queried in the Freelancer container
- current_freelancer is passed to child components
- formatting (concatenating names, renaming attributes) is done in child components
- /freelancer/:id now displays that freelancer's information page
- Main component (default app view) needs a rethink.
- Freelancer container may need to be factored out into multiple
  components depending on behavior desired.
- Components rendering freelancer information now only render
  once a freelancer has been retrieved from the store.
- Freelancer.loading_freelancer added to store to track this.
- Blank view can be replaced later with a loading message/spinner.
- Cosmetic code fixes added as well.
Copy link
Owner

@ortonomy ortonomy left a comment

Choose a reason for hiding this comment

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

I couldn't get the freelancer data to display when I ran it. So I looked at the code and there are a few fixes needed with the query and the logic to make the call. Let me know if you need any support.

flEmploymentStatus
}
}
}
Copy link
Owner

Choose a reason for hiding this comment

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

This query has two issues:

  • it's a statically coded UUID which means I can't test because UUIDs are different on each instances. replace with ${id}
  • the query returns a 400 bad request response.

The correct query is something like this:

{
  allFreelancers(condition: {
    flId: "0ba6e94f-1956-4a94-b079-bc29fde23374"
  }) {
    edges {
      node {
        flId
        flLastName
        flFirstName
        flLocation
        flTimezone
        flIsNativeSpeaker
        flPrimaryLanguage
        flEmploymentStatus
      }
    }
  }
}

the response you get back is something like:

{ data { allFreelancers:  { edges:  [ { node: { ... } , ... ] } } }

Note the array called edges and then each freelancer is a node inside the array.

})
.then ( () => {
done();
});
Copy link
Owner

Choose a reason for hiding this comment

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

The 2nd API call in the process hook of the logic for action FREELANCER_INFO is redundant. You can remove it.

I didn't find Redux Logic intuitive at first either, but after the validate hook runs, logic lets the action go through to the reducer. the process hook is run after.

So in effect your first action (in this case FREELANCER_QUERY) is the initiator of the flow of retrieving data, but and the API call is a side effect.

The actions as far as redux is concerned look like this:

  1. FREELANCER_QUERY =>
  2. intercepted by VALIDATE =>
  3. OK? => (*)
  4. REDUCER =>
  5. END
    but at the point of (*) Logic passes to its own process hook where you can do async stuff and dispatch other actions

hence...

process => API => result => FREELANCER_INFO(result).

Technically, you don't even need the logic for FREELANCER_INFO action, but it's good to validate it and make sure the payload is passed.

.then ( data => {
if ( data ) {
Debug.log('[action:process:freelancer_info] FREELANCER_QUERY', data);
dispatch(actions.freelancerInfo(data));
Copy link
Owner

Choose a reason for hiding this comment

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

Change this line to use the correct response from the API:

dispatch(actions.freelancerInfo(data.allFreelancers.edges[0]));

(based on API change below)

...state,
loading_freelancer: false,
current_freelancer: {
...action.payload.allFreelancers.nodes[0]
Copy link
Owner

Choose a reason for hiding this comment

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

You'll need to change this to only use action.payload.node if you follow my suggestion about action dispatch above.

Copy link
Owner

@ortonomy ortonomy left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes. checked and it all seems to work!

@ortonomy ortonomy merged commit 83f17ba into development Mar 5, 2018
@ortonomy ortonomy deleted the live-freelancer-data branch March 5, 2018 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants