Skip to content

Conversation

@stephensanchez
Copy link
Contributor

Tests verified, manually tested.

@cpennington Regarding our conversation this morning around the CSV action to pull anonymous user IDs, this PR would change the CSV to include the course-specific IDs.

@Lyla-Fischer This change alters the contract of what is returned by this instructor dashboard action, is there any risk of impacting staff / instructors who may rely on the old Anonymous IDs?

@wedaly

@wedaly
Copy link
Contributor

wedaly commented May 13, 2014

I'm concerned about this change. Researchers might be relying on the anonymous IDs in these spreadsheets to be consistent over time. I'm also pretty sure there are XModules that are still using IDs that are not unique to a particular course.

I think adding an extra column would be safer.

@Lyla-Fischer
Copy link

++ @wedaly

@stephensanchez
Copy link
Contributor Author

I could definitely add another column for Course Specific Anonymized User IDs. It means I'll likely have to make separate queries.

@cpennington does that sound good to you?

Copy link
Contributor

Choose a reason for hiding this comment

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

You can actually just put the return value in these definitions:

@patch.object(instructor.views.api, 'anonymous_id_for_user', Mock(return_value=41))

@stephensanchez stephensanchez changed the title WIP: Changing Student Anon ID Dump to return Course Specific IDs. Changing Student Anon ID Dump to return Course Specific IDs. May 14, 2014
@stephensanchez
Copy link
Contributor Author

  • Removed Course ID column (redundant)
  • Updated tests
  • Added Course Specific Anonymized User ID column, instead of changing current column.

@stroilova
Copy link
Contributor

Hello, may I ask @brianhw / @rocha / @mulby whether this new format is parallel to the Anonymous ID info we are currently exporting as part of the research data packages?

  • do we export both the global and course-specific anonymous user ids ? user-id-map seems to contain only one of the two, but I remember we had to patch in the other before?
  • should the format of the file we provide (user-id-map ) match the format of the file available from the Instructor Dashboard to preempt future questions?

Thank you

@mulby
Copy link
Contributor

mulby commented May 14, 2014

@stroilova - we are not currently exporting this ID. See https://edx-wiki.atlassian.net/browse/AN-320.

We export database tables in a very specific format, I doubt we would want to change that for this reason. Also we typically try to mirror the table schema as closely as possible, so we typically aren't joining in columns from other tables etc, which would be necessary to add this information to the current file.

@wedaly
Copy link
Contributor

wedaly commented May 14, 2014

👍

stephensanchez pushed a commit that referenced this pull request May 14, 2014
Changing Student Anon ID Dump to return Course Specific IDs.
@stephensanchez stephensanchez merged commit 0a6ea26 into master May 14, 2014
@stephensanchez stephensanchez deleted the sanchez/course-anonymous-ids-in-csv branch May 14, 2014 19:04
@stroilova
Copy link
Contributor

Ah ok. That makes sense, thank you @mulby .

On Wed, May 14, 2014 at 3:04 PM, Stephen Sanchez
notifications@github.comwrote:

Merged #3703 https://github.com/edx/edx-platform/pull/3703.


Reply to this email directly or view it on GitHubhttps://github.com/edx/edx-platform/pull/3703#event-121162966
.

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.

7 participants