tl-its-umich-edu / my-learning-analytics

My Learning Analytics (MyLA)
Apache License 2.0
36 stars 39 forks source link

issue_1573 added user_id column from resource_access table as an index #1579

Open zqian opened 1 month ago

jonespm commented 1 month ago

Hi Zhen, I started looking into this but didn't finish. The DB's suggested that we also have an index on the user_id on the user and submissions table.

But as I looked more into it, I thought that this probably should just be a foreign key reference instead. And if it had been this wouldn't be an issue. However with a foreign key you have to decide what to do on_delete. It would probably be SET_DEFAULT(or SET_NULL) or DELETE. It seems like it would be fine if a user was deleted to cascade delete this.

As I thought about it more I also didn't know if we even need the autofield on the User table since it's not even used for anything and user_id here is the primary key. All that kind of started to feel like a growing issue so I didn't get it implemented.

I still think this should be a foreign key across all user_id referenced tables rather than just an index. But I guess just making this (and the user_id fields in submission and user also index) should be fine. That's basically like making it a foreign key with the option DO_NOTHING.

zqian commented 1 month ago

I like the foreign key approach with cascade delete option for better referential integrity. I will give that a try.

As for the auto ID field in User table, we will address that issue later

jonespm commented 1 month ago

Yeah, my WIP had this. We probably don't need to FK all of these columns up but it seemed like it would work. user_id was the one identified as the problem here. This is untested and FK on those other 2 might cauase issues though it seems correct.

+class ResourceAccess(models.Model):
+    id = models.AutoField(primary_key=True, verbose_name="Table Id")
+    resource_id = models.ForeignKey(Resource, on_delete=models.CASCADE, to_field='resource_id', db_column='resource_id')
+    course_id = models.ForeignKey(Course, null=True, default=None, on_delete=models.CASCADE, db_column='course_id')
+    user_id = models.ForeignKey(User, on_delete=models.CASCADE, to_field='user_id', db_column='user_id')
+    access_time = models.DateTimeField(verbose_name="Access Time")
+
+    def __str__(self):
+        return f"Resource {self.resource_id} accessed by {self.user_id}"
+
+    class Meta:
+        db_table = 'resource_access'