DASSL / ClassDB

An open-source system to let students experiment with relational data
https://dassl.github.io/ClassDB/
Other
7 stars 2 forks source link

File addUserMgmtCore.sql defines objects used only by reco scripts (WE) #217

Closed smurthys closed 6 years ago

smurthys commented 6 years ago

The script file addUserMgmtCore.sql defines objects not used by any core component.

I propose the following changes:

EDIT: See comment below on a resolution that leaves the file as is, and addresses the issue by just adding comments.

wildtayne commented 6 years ago

I think the difficulty with this change is that ClassDB.User depends on both activity tables. We could switch to a modular setup where the new suggested scripts execute ALTER VIEW statements to add the relevant query over each activity table.

Additionally, the Activity Views rely on the activity columns in Class DB.User, so Class DB.User would need blank/dummy columns in case activity logging components are not installed.

smurthys commented 6 years ago

Thanks @srrollo for refreshing my memory. I do now recall this before M2 and deciding to postpone due to time constraint. I suppose we have to do that again. However this time we at least have an issue logged.

smurthys commented 6 years ago

For M3, I suggest we add comments to the file in question to clearly say why it is a core component. Without such comments, this issue will come up repeatedly and consume valuable research/discussion time.

Just adding comments might even be the long-term resolution given the complexity introduced by the alternative: conditionally altering views and adding dummy columns would significantly reduce code maintainability.