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

Text responses tab: add textResponsesRedux #22304

Merged
merged 12 commits into from May 10, 2018
Merged

Conversation

maddiedierker
Copy link
Contributor

@maddiedierker maddiedierker commented May 10, 2018

In order to convert the "text responses" tab in teacher dashboard to react, we will want student text response data to be available in redux. This PR creates the reducer with a way to pull responses into redux from dashboardapi and controls loading states since the former action is asynchronous.

Since a teacher will always be viewing text responses in the scope of a section and a script, theresponseData object in redux state looks as follows (some response properties removed for clarity):

responseData = {
  scriptId: [{
    question: "Who's your favorite programmer?",
    response: "Grace Hopper!",
    studentName: "Maddie",
    studentId: 1
  }]
}

responseData is then cleared when the teacher switches sections. This structure 1) allows the reducer to keep track of which scripts it has already loaded and will not re-request that data from the server if it already has it, and 2) makes rendering the response table more straightforward.

…hanged and scope by scriptId rather than sectionId+scriptId
Copy link
Contributor

@caleybrock caleybrock left a comment

Choose a reason for hiding this comment

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

Looks good to me!

const FINISH_LOADING_RESPONSES = 'textResponses/FINISH_LOADING_RESPONSES';

// Action creators
export const setTextResponses = (sectionId, scriptId, responseData) => ({ type: SET_TEXT_RESPONSES, sectionId, scriptId, responseData});
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this is pretty long - should be broken up into two.

url: PropTypes.string.isRequired
});

// Initial state of textResponsesRedux
Copy link
Contributor

Choose a reason for hiding this comment

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

could be useful to have a comment explaining that responseData is an object where the key is a script, and the value is etc.

In progress, we appended "byScript" to our names to make it a bit more clear: https://github.com/code-dot-org/code-dot-org/blob/staging/apps/src/templates/sectionProgress/sectionProgressRedux.js#L153

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.

None yet

2 participants