Handle changes to the Canvas IDs from long ids to short ids with the switch to Canvas Data 2

[jennlove-um]

With the change to CD2 in the UDP, the Canvas IDs provided are the short IDs, not the long IDs. MyLA queries need to handle this change for new and existing data.

• Determine what needs to be changed in MyLA to accommodate this, e.g., database schema, chron
• Update values in tables as needed

This will require thorough testing to verify that everything is working as expected after the change.

[binkmywink]

Hi! I am in EECS 481 and am working on contributing to an open sourced GitHub project. My partner (niurcaq@umich.edu) and I (ltheresa@umich.edu) were interested in this task. We were wondering what the status on this issue was and how we could get started on it.

[jennlove-um]

@binkmywink This issue requires a couple of other issues to be resolved first before work on this issue can begin. It's not one that we would recommend starting on at this point, but thanks for your interest!

[pushyamig]

I will start my research w.r.t the issue this week.

[pushyamig]

My Research on this issue:

1. course, term table need a migration change, need to remove canvas_id which is the short canvas Id of the respective table
2. Course table has both long/short course id's. So Id is longId's and canvas_id is shortID. Id column need to have shortId. converting is going to be a manual change. Since all the subsequent tables assignment, resources, resources_access, 'submission, 'assignment_groups, 'assignment_weight_considerationreference course id from this table (i.e and co_km.lms_int_id IN UNNEST(@course_ids)). We might need to write some SQL query to covertId` columns longID to shortID before we run the migration. So all respective schools might need to do this manual step as which would be inconvenient. I don't know if there is another easy fix for this.
3. Canvas Live events still send the data as Canvas LongID. So the queries need to convert the LongId's to ShortId's before we load it again. I think we might need to do an one time delete and add data instead of incremental approach we have now.
4. We still need to keep the CANVAS_DATA_ID_INCREMENT in the env.hjson since Event data need conversation
5. All cron queries need to change to pull shortId from context_store which is not a complicated
6. Unizin do not know if they really going to remove the LongID's yet. Comment from Tracy

 For now, we are just beginning the "discussion stage" about the long IDs and when (or even if) we want to remove them from the keymap.
We will keep you updated as the discussion progresses. For now, though, we have no plans to remove them in the near term!

[pushyamig]

I reached out to Unizin since I want to understand what they are thinking w.r.t to LongId removal when Canvas Live events still Send the event with Canvas LongId's. The response seems to be generic from Unizin. This issue will do a major change to Cron process and involved lot of testing. So we need to think if we want to do this now or wait a bit

[jonespm]

Thanks, for the research! Based on Unizin continuing to support this and the work involved I'd probably lower the priority of this and take it out of this release until/if it's needed. Reading about this more we might not ever implement this if Unizin doesn't remove this.

I think back last year we were more concerned as the CD2 didn't have these long IDs but they kept them in UDP context_store for consistency with CD1. I think there's some advantages to retaining this prefix. I agree that if we removed it we'd likely have large migrations and schema changes to the data model of MyLA.

As far as like #4 above, we'd could probably move that CANVAS_DATA_ID_INCREMENT into the course table, or in a new table. This would need to keep the course id's unique somehow, either by that value or with a composite key that relates to the LTI tool. This would be so we could future feature ideas and data that isn't in the short format like having synthetic data in the database alongside regular data, and possibly also be multi-tenant. This sounds be a lot to work through.

Thanks for checking!

[pushyamig]

Recent comment from Unizin about removal of LongId's.

After a brief discussion today with the team, we've determined that there are no plans to remove them!

We could close this issue if we decide we will stick with current design.

If we move the Cron from Context Store to CD2 schema (if thinking about Supporting Non-Unizn Schools)then we can think about this. But that could be separate issue

[jennlove-um]

Thanks for the research and comments! Due to the response from Unizin, there isn't a need to make this change. I'm going to close this issue. If we decide to move to using CD2 or otherwise changing the queries, we will create a new issue.