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

Teacher Feedback - Fix query parameter parsing #23870

Merged
merged 2 commits into from Jul 24, 2018

Conversation

epeach
Copy link

@epeach epeach commented Jul 23, 2018

Fixing a silly mistake where I was parsing the query parameters based on their location in the string vs. the key. Updated to specifically looking for the value with the key 'user_id'.

const studentId = search.split('&')[1].split("=")[1];
//Pull the student id from the url
let studentId = "";
search.split('&').map(item => { studentId = item.includes('user_id') ? item.split('=')[1] : studentId; });
Copy link
Contributor

Choose a reason for hiding this comment

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

A couple nits here:

  1. Since we only care about one of the items I find map less intuitive than find here.
  2. This doesn't check word boundaries; suppose in the future we added pair_user_id to the querystring for this page, this code would be ambiguous about which id we were retrieving.

I have two suggestions. The first is to use the query-string package we already depend on.

import queryString from 'query-string';
const studentId = queryString.parse(window.location.search).user_id;

The second, if you prefer not to depend on that library, would be to use Array.protoytpe.find and RegExp to locate and extract the value.

const studentId = search.split('&').find((pair) => {
  const match = pair.match(/^user_id=(.*)$/);
  return match && decodeUriComponent(match[1]);
});

Using the library is probably the more dependable way to go.

Copy link
Contributor

@islemaster islemaster left a comment

Choose a reason for hiding this comment

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

👍

@epeach epeach merged commit c4fecb9 into staging Jul 24, 2018
@epeach epeach deleted the teacher-feedback-fix-student-id branch July 30, 2018 16:57
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

Successfully merging this pull request may close these issues.

None yet

2 participants