-
Notifications
You must be signed in to change notification settings - Fork 0
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
Project listing #1
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a comment about getting the correct project_contents
id for each project.
@@ -26,12 +26,12 @@ const projectContentsReducer = (state = initialState, action) => { | |||
}; | |||
|
|||
// Action Creators | |||
const fetchProjectContents = () => { | |||
const fetchProjectContents = (project_id) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're requesting project_contents
with project_id
, which isn't right. project_contents
id is needed, which available in project links
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad, this should actually be requesting a list of all ProjectContents
resources for a given project, then listing them by language.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed now.
src/ducks/contents.js
Outdated
@@ -31,7 +31,10 @@ const fetchProjectContents = (project_id) => { | |||
dispatch({ | |||
type: FETCH_PROJECT_CONTENTS, | |||
}); | |||
apiClient.type('project_contents').get(project_id) | |||
const query = { | |||
project_id: project_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shorthand property is nicer perhaps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed.
Only display the first for now.
d98ad5b
to
519aec7
Compare
LGTM! |
Adds a basic project listing, linking each project up to the project contents view.