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

Limit resources access view to show only events for enrolled students in the course #764

Closed
jennlove-um opened this issue Oct 18, 2019 · 16 comments · Fixed by #1395
Closed
Assignees

Comments

@jennlove-um
Copy link
Contributor

jennlove-um commented Oct 18, 2019

##Expected behavior:
Resource access events for enrolled students only should be included in the percentage of people who have viewed an item. Also, when anyone with any other role in the course views the page, they should not see blue bars to indicate which files they've viewed.

Describe the bug:

Currently, with my login, I'm seeing files I viewed in the course showing up as blue in the visualization. I am not a student in any courses, but I am a Teacher. My file access should not be considered in the visualization since I am not a student.

Steps to Reproduce :

  1. As someone who is not a student go to a Canvas course that is in MyLA
  2. Access a file in the course
  3. View the resources view to see that the file shows up as a blue bar instead of gray

Additional context :

This visualization is about student behavior, so it should limit the display to include access events for only students who are enrolled in the course.

@jennlove-um jennlove-um added the 🪳 bug Something isn't working label Oct 18, 2019
@project-bot project-bot bot added this to To do in MyLA-Default-Project Oct 18, 2019
@jpyan
Copy link
Contributor

jpyan commented Nov 27, 2019

Hey I'm trying to look into this bug yet I can't seem to see any blue file from the admin side for any courses. For example, for course id 362855, it shows blue on student's view, but no blue in admin view. Is it already being fixed? If not, would you provide a more specific example to reproduce that bug? (Screenshots would be helpful too)

@jennlove-um
Copy link
Contributor Author

Thanks for asking the question - the issue isn't really with the admin view. That was my first pass at understanding what was going on. I'll update the description. But I've also put some more detail below.

For this bug, the issue is that only users with an enrollment type of "StudentEnrollment" in the user table should have their resource access events get counted and included in the visualization.

Replicating this requires going into Canvas as someone with a different role on a course (TA, Teacher, Designer, etc.) and clicking on a file. We don't want the clicks from those enrollment types to get counted in this visualization. So if I'm a teacher, and I click on files then go to MyLA, I shouldn't see any bars show up as blue since my file access doesn't count.

@jpyan
Copy link
Contributor

jpyan commented Nov 27, 2019

Thanks for your reply. So with "requires going into Canvas as someone with a different role on a course (TA, Teacher, Designer, etc.) and clicking on a file", does that mean if I'm not a teacher nor some other role instead of students, I can't reproduce this bug?

@jennlove-um
Copy link
Contributor Author

You wouldn't necessarily have to go into Canvas. But you do need to have a user in the user table that has an enrollment_type other than StudentEnrollment. For example, TeacherEnrollment, TaEnrollment.

You also have to have an entry in the resource access table for that same user.

If you have a user with that type of enrollment who has accessed a resource, then you could masquerade as that user to see the issue through the UI. However, I don't think that there are any users with that kind of enrollment type in the demo data. So you would have to edit the data to set this up.

@wangyis
Copy link

wangyis commented Dec 3, 2019

Could you clarify where to set this up and changing the enrollment type? Do you mean adding groups in the root admin panel? Or do we need to hand-code in somewhere else?

@jennlove-um
Copy link
Contributor Author

In the user table, there is a column called "enrollment_type." For the sample data, all rows have "StudentEnrollment" in that column. You could manually change the enrollment type in that column for one of the users to "TeacherEnrollment" to set that person up as a teacher instead of a student.

You shouldn't need to add any groups or make any changes on the admin panel via the UI. You can manually change the data in the database for one user's row.

Just make sure that the user you pick to change also has at least one row in the resource_access table to indicate that they accessed a file as well.

The condition you need to test this change would be a user that has accessed a file and that has a non-StudentEnrolllment type in the user table.

Once you have that condition set up, you can test the code that you write to make sure only student file access events are included in the Resources Accessed view. The user you changed should not see any files accessed and should not have their access events included in the percent of students who accessed a file.

@wangyis
Copy link

wangyis commented Dec 4, 2019

I am having a struggling time finding the place where the project draws the chart about the resource accessed and the place where the python program accessing the SQL data about the resource accessed. Could you give us more details about the project?

@wangyis
Copy link

wangyis commented Dec 4, 2019

I want to add a condition to judge if the enrolment type is 'student' before the website makes the histogram about the resource accessed. Hope this information helps!

@jennlove-um
Copy link
Contributor Author

@pushyamig @zqian @jonespm or @ssciolla - Can one of you help out with the questions on this one? Once we're in code-land, we're out of my area of expertise!

@pushyamig
Copy link
Contributor

@wangyis, I Am assuming you can reproduce the bug with @jennlove-um instructions.
The Python code to look at is views.py and function resource_access_within_week
The way the resource access works is but will get the class aggregate data of resources accessed and then the user personal resources accessed data.

When I checked the code where we pull the aggregate data from L229-237 that is explicitly querying based on student enrollment data. When you go further L286-L291 the comment clearly states that user personal resource access data is queriyed, I think we need to make a check if user is student enrollment or not. That may be the reason why jennifer was seeing her resouce access data in the course.

@pushyamig
Copy link
Contributor

On the other end I was trying to thing why not fix this in cron.py when we are pulling the data from Big query. Only pull student data based on the memership.roles attribute in the caliper data. Here is the BigQuery query

SELECT 'canvas' AS resource_type, 
CAST(SUBSTR(JSON_EXTRACT_SCALAR(event, '$.object.id'), 35) AS STRING) AS resource_id, 
CAST(SUBSTR(JSON_EXTRACT_SCALAR(event, '$.membership.member.id'), 29) AS INT64) AS user_id, 
CAST(SUBSTR(JSON_EXTRACT_SCALAR(event, '$.group.id'),31) AS INT64) AS course_id, 
JSON_EXTRACT_SCALAR(event, '$.object.name') as name, 
datetime(EVENT_TIME) as access_time,
event
FROM event_store.events 
where JSON_EXTRACT_SCALAR(event, '$.edApp.id') IN UNNEST(['http://m.canvas.umich.edu/', 'http://umich.instructure.com/']) and 
type = 'NavigationEvent' 
and SUBSTR(JSON_EXTRACT_SCALAR(event, '$.object.id'),24,10) = 'attachment' 
and JSON_EXTRACT_SCALAR(event, '$.action') = 'NavigatedTo' 
and JSON_EXTRACT_SCALAR(event, '$.membership.member.id') is not null 
** JSON_EXTRACT_SCALAR(event, '$.membership.roles[0]') as role = 'Learner',** 
and SUBSTR(JSON_EXTRACT_SCALAR(event, '$.group.id'),31) IN UNNEST(['17700000000307712'])

JSON_EXTRACT_SCALAR(event, '$.membership.roles') as role = 'Learner', $.membership.roles this returns a list my query is not accurate I need to figure how to match for Learner from the list but the idea works.
"roles":[
"Learner",
"Observer"
]

@wangyis you can't have access to this system called bigquery where data is stored, I am exploring the options the best way to fix this.

@jennlove-um
Copy link
Contributor Author

With the addition of #1332 in the next release, this needs to be revisited. We should double-check files and pages to make sure we aren't including ones that only teachers or TAs have accessed.

@ssciolla
Copy link
Contributor

Steps I used to test #1395:

  1. Set up MyLA with a test course.
  2. Run cron based on master.
  3. Run query to get count of resource access events and identify teacher events. If teacher events aren't present, find another course.
    select 
    from resource_access ra
    join user ue
        on ra.user_id=ue.user_id
        and ra.course_id=ue.course_id;
    
  4. Switch to issue branch.
  5. Update data last updated for course to be null as suggested by @zqian:
    update course set data_last_updated=null;
    
  6. Run cron again.
  7. Use query again from 3) and check that filtering has occurred.

@jennlove-um
Copy link
Contributor Author

jennlove-um commented Aug 24, 2022

We need to determine how to remove the old events for Teachers and TAs for prior terms.

Either set the last update date to null to pull in all the data in one time or write SQL to delete the Teacher and TA events in the database.

Recommendation: Add an option to the Django Admin screens to reset the course refresh dates to null to refresh the events data for all courses.

MyLA-2022.02.01 automation moved this from In progress to Review/QA Aug 26, 2022
zqian added a commit that referenced this issue Aug 26, 2022
Co-authored-by: Sam Sciolla <35741256+ssciolla@users.noreply.github.com>
zqian added a commit that referenced this issue Sep 19, 2022
@zqian zqian removed this from Review/QA in MyLA-2022.02.01 Sep 19, 2022
@zqian zqian added this to To do in MyLA-2022.02.01 via automation Sep 20, 2022
@zqian zqian moved this from To do to In progress in MyLA-2022.02.01 Nov 15, 2022
zqian added a commit that referenced this issue Nov 29, 2022
… in the course #764 (#1460)

Co-authored-by: Sam Sciolla <35741256+ssciolla@users.noreply.github.com>
@zqian zqian moved this from In progress to Review/QA in MyLA-2022.02.01 Dec 8, 2022
@zqian zqian moved this from Review/QA to Review/QA - DEV in MyLA-2022.02.01 Dec 8, 2022
@zqian zqian moved this from Review/QA - DEV to Review/QA in MyLA-2022.02.01 Dec 8, 2022
@jennlove-um
Copy link
Contributor Author

Testing passes in beta. Events for TeacherEnrollments are not displaying in the Resources Accessed view.

@jennlove-um jennlove-um moved this from Review/QA to Done in MyLA-2022.02.01 Dec 15, 2022
@jennlove-um
Copy link
Contributor Author

This passes in MyLA test. Events for TeacherEnrollments are not displaying in the Resources Accessed view. Events for StudentEnrollments are displaying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

7 participants