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

Potential for duplicate resource identifiers (Pages visualization) #1332

Closed
jlongland opened this issue Oct 19, 2021 · 9 comments · Fixed by #1391
Closed

Potential for duplicate resource identifiers (Pages visualization) #1332

jlongland opened this issue Oct 19, 2021 · 9 comments · Fixed by #1391
Assignees
Labels
config change needed Changes are needed/included that may affect configuration files 📈 enhancement New feature or request

Comments

@jlongland
Copy link

We received feedback from a pilot instructor that Canvas wiki page views would be more useful than file views for the Resources Accessed dashboard in their course. It makes sense given that the bulk of their content is in pages. I decided that I would add an additional query to RESOURCE_ACCESS_CONFIG. There's a slight issue with this given that Canvas Live Events lack the attributes to generate a link to the resource (and uses a different pattern than files). I was going to workaround this by passing an empty string from views.py for the URL (and look at a better solution in the future).

But as I was testing, I realized that the resource.resource_id is populated with whatever identifier is provided in the query of the source learning tool. Not ideal when loading resources from multiple source learning tools. I suppose the odds of an identifier collision are low when the source tools are likely using different identifier formats? But when we’re loading resources from the same source that use the same pattern for identifiers (e.g. bigint in the case of Canvas) it seems like we run the risk of an identifier collision? Which seems like an issue given the constraint on the model.

So I checked our data and it looks like this will be a problem:

select count(*)
from file_dim
join wiki_page_dim 
on file_dim.id = wiki_page_dim.id

1330848 rows

I was just wondering if this has come up in any previous discussions? I searched issues but didn't come across anything about this specifically. I was wondering if it would make sense to have a generated resource.id and a resource.source_id?

@jennlove-um jennlove-um added this to To do in MyLA-Default-Project via automation Oct 20, 2021
@jennlove-um jennlove-um added the 📈 enhancement New feature or request label Oct 20, 2021
@jlongland
Copy link
Author

@jennlove-um I'm not sure this is necessarily an enhancement? The potential for identifier collision feels like more of a bug. If two source learning tools are using the same approach for identifiers (ex. big integer), there will be an identifier collision.

My inclination is that there should be a MyLA resource identifier as resource.id and the identifier from source learning tools should be something like resource.source_id But it's possible I'm misunderstanding the situation here. Curious if @jonespm or @zqian have comments?

@zqian
Copy link
Member

zqian commented Mar 10, 2022

@jlongland sorry we did not reply earlier. This is a valid concern for id collision. We did not have the problem here in UM because we are not including the wiki usage data.

Somehow, I think we can solve this problem with existing db schema: we just need to use different value for resource_type field in the MyLA resource table.

Currently, the resource_type is set to "canvas" for Canvas files. We can use "canvas" for Canvas file record, and "canvas_pages" for wiki page, etc. WDYT?

@zqian zqian added this to To do in MyLA-2022.02.01 via automation Jul 6, 2022
@jennlove-um
Copy link
Contributor

For the Resources Accessed view, we would also need to add the filter for the type "Canvas pages" to differentiate them from the files and videos. We will also need to identify an icon for the pages to differentiate them.

@zqian zqian removed this from To do in MyLA-Default-Project Jul 7, 2022
@zqian zqian moved this from To do to In progress in MyLA-2022.02.01 Jul 8, 2022
@jennlove-um jennlove-um changed the title Potential for duplicate resource identifiers Potential for duplicate resource identifiers (Pages visualization) Jul 22, 2022
@jennlove-um
Copy link
Contributor

The display should be limited in Resources Accessed to show only pages that are accessible to students. Pages that are accessed by teachers only should not be included. For example, pages that are not published should not be included in the view.

@zqian
Copy link
Member

zqian commented Aug 17, 2022

The need of "reporting only the student-initiated page access events" will be handled by #764 , alongside with the data types of resource access and video access. So I would suggest we focus on the "Pages view" event query for this issue. @jonespm and @jennlove-um ?

@jennlove-um
Copy link
Contributor

From a functional point of view, I'm only concerned that both issues get done so that only student events are displayed in the view. So whichever way it makes sense to do the coding is fine with me.

MyLA-2022.02.01 automation moved this from In progress to Review/QA Aug 20, 2022
@zqian zqian added the config change needed Changes are needed/included that may affect configuration files label Aug 20, 2022
@jennlove-um
Copy link
Contributor

Pages are displayed in the Resources Accessed in testing in Beta. Testing for this issue passes.

Note: Mivideo events are not there, but for the moment, that appears to be a separate issue.

@jennlove-um jennlove-um removed this from Review/QA in MyLA-2022.02.01 Sep 20, 2022
@jennlove-um jennlove-um added this to To do in MyLA-2022.01.03 via automation Sep 20, 2022
@pushyamig pushyamig moved this from To do to Review/QA in MyLA-2022.01.03 Sep 20, 2022
@jonespm jonespm moved this from Review/QA to Review/QA - DEV in MyLA-2022.01.03 Sep 20, 2022
@zqian zqian moved this from Review/QA - DEV to Review/QA in MyLA-2022.01.03 Sep 21, 2022
@jennlove-um
Copy link
Contributor

Testing passes in beta for the MyLA 2022.01.03 release.

@jennlove-um jennlove-um moved this from Review/QA to Done in MyLA-2022.01.03 Sep 21, 2022
@jennlove-um
Copy link
Contributor

Testing passes in test for the MyLA 2022.01.03 release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
config change needed Changes are needed/included that may affect configuration files 📈 enhancement New feature or request
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants