Ongoing Issue: Unit and Integration Testing

[JRustyHaner]

@imrryr @MegaGeese I'm going to put notes in here regarding my efforts to make unit tests and integration tests for mofacts. Usually they will be questions I might have about specs, or observations from running the tests. I'm going through each function, and I've already found something that is interesting. Sorry if this is a lot, but I want to harden mofacts a bit.

Methods.js, Line 2655 getAccessableTDFSForUser and Line 2662 getAssignableTDFSForUser Question, why is there a difference between 'Accessible' and 'Assignable' in this context. If something is accessible, it should be assignable, right? The getAccessableTDFSForUser function is only used in data download, and getAssignableTDFSForUser is only used in assignments. These are functionally the same thing?

[imrryr]

Yes I can't think of a situation where a teacher would be able to assign content and then not be able to access data for it. It seems redundant to have both objects. Thanks for your work Rusty.

On Tue, Sep 10, 2024, 9:45 AM JRustyHaner @.***> wrote:

Assigned #1563 https://github.com/memphis-iis/mofacts/issues/1563 to @imrryr https://github.com/imrryr.

— Reply to this email directly, view it on GitHub https://github.com/memphis-iis/mofacts/issues/1563#event-14200316123, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADDLPK5RO2TLSEWGRLTSEFLZV4AXBAVCNFSM6AAAAABN65SZOWVHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJUGIYDAMZRGYYTEMY . You are receiving this because you were assigned.Message ID: @.***>

[JRustyHaner]

@imrryr The change then would be how it's stored. Accessible is stored in the user's database, Assignable is in the tdf's database. So, the change would be moving accessible (probably change to accessibleBy, for clarity) to a tdf spec, and remove the assignable flag entirely. Thoughts?

[imrryr]

I don't understand how Assignable can even be in the tdf database, since assignability is relative to the USER that uploaded the file or was given access. Does each tdf have a list of users that it is assignable for?

On Tue, Sep 10, 2024 at 2:46 PM JRustyHaner @.***> wrote:

@imrryr https://github.com/imrryr The change then would be how it's stored. Accessible is stored in the user's database, Assignable is in the tdf's database. So, the change would be moving accessible (probably change to accessibleBy, for clarity) to a tdf spec, and remove the assignable flag entirely. Thoughts?

— Reply to this email directly, view it on GitHub https://github.com/memphis-iis/mofacts/issues/1563#issuecomment-2341884224, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADDLPK3KQGPU6D6AV4QA6VLZV5ECHAVCNFSM6AAAAABN65SZOWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBRHA4DIMRSGQ . You are receiving this because you were mentioned.Message ID: @.***>

-- Philip I. Pavlik Jr. @.*** http://optimallearning.org/

[JRustyHaner]

@imrryr inside the tdf database, each tdf has an accessors list and an owner which are added in there after upload.

[JRustyHaner]

And yes, each user has a list of tdfs of which they are assigned. I guess the issue is that the terminology in the script, which I am guessing you didn't approve. The get accessible tdfs function looks to find which tdfids the user has in their user.accessed list. The get assignable tdf returns a list of tdfids where the user is the owner or in the tdf.accsessor list.

I guess there is a subtle difference there. The user.accessed list should probably be named user.assigned and contain the tdfids of their assignments. I know this is superfluous because we are moving to LTI in the future Since we have a lot of terminology that you didn't probably agree to in the code, it might be important to address these instances in where data is stored and what the data is named.

[imrryr]

OK, not sure all of what you are saying there. Basically, I think you are right that there is a redundancy; I would not expect any information stored in the tdfs database themselves. Why can't we simply have a list of tdfs for each user. So it would be a simple list of pairs for each user, the tdfidentifier and the word "owner" or "accessor". Where they are the owner if they are the uploader and accessor if given permission.

I can't think of a case where that is not sufficient. Am I missing something?

On Tue, Sep 10, 2024 at 6:21 PM JRustyHaner @.***> wrote:

And yes, each user has a list of tdfs of which they are assigned. I guess the issue is that the terminology in the script, which I am guessing you didn't approve. The get accessible tdfs function looks to find which tdfids the user has in their user.accessed list. The get assignable tdf returns a list of tdfids where the user is the owner or in the tdf.accsessor list.

I guess there is a subtle difference there. The user.accessed list should probably be named user.assigned and contain the tdfids of their assignments. I know this is superfluous because we are moving to LTI in the future Since we have a lot of terminology that you didn't probably agree to in the code, it might be important to address these instances in where data is stored and what the data is named.

— Reply to this email directly, view it on GitHub https://github.com/memphis-iis/mofacts/issues/1563#issuecomment-2342327014, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADDLPK5RPFHXME5BLKXPEHLZV55I7AVCNFSM6AAAAABN65SZOWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBSGMZDOMBRGQ . You are receiving this because you were mentioned.Message ID: @.***>

-- Philip I. Pavlik Jr. @.*** http://optimallearning.org/

[JRustyHaner]

I guess the only worry I would have is a use case like the following:

Given: -Tdfs have owners that can always access and assign to classes -.... have accessors that can access and assign to classes -Users that are not students can only access public or assigned Tdfs.

Cases: -Owner can add a Teacher as Accessor -Accessors can assign a Tdf To a Student. -Students can access Tdfs they are assigned. -Owner doesn't add Teacher as Accessor, Tdf cannot be assigned by that Teacher -Owner adds a Teacher as Accessor, but Teacher doesn't assign it to Student, Student cannot access the Tdf.

[imrryr]

I can't follow this. I was saying owners have tdfs. I was saying tdfs should not have owners. Also, we aren't talking about student functions; students are not "assigned" anything in the current system. Unless some facility I don't know about was built without consulting me, students are irrelevant to this discussion.

On Wed, Sep 11, 2024 at 4:47 PM JRustyHaner @.***> wrote:

I guess the only worry I would have is a use case like the following:

Given: -Tdfs have owners that can always access and assign to classes -.... have accessors that can access and assign to classes -Users that are not students can only access public or assigned Tdfs.

Cases: -Owner can add a Teacher as Accessor -Accessors can assign a Tdf To a Student. -Students can access Tdfs they are assigned. -Owner doesn't add Teacher as Accessor, Tdf cannot be assigned by that Teacher -Owner adds a Teacher as Accessor, but Teacher doesn't assign it to Student, Student cannot access the Tdf.

— Reply to this email directly, view it on GitHub https://github.com/memphis-iis/mofacts/issues/1563#issuecomment-2344760180, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADDLPK47SBQMI46LF4N52UTZWC257AVCNFSM6AAAAABN65SZOWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNBUG43DAMJYGA . You are receiving this because you were mentioned.Message ID: @.***>

-- Philip I. Pavlik Jr. @.*** http://optimallearning.org/

[JRustyHaner]

Tdfs do currently have owners, this is a protection to prevent teachers from deleting tdfs and data that do not belong to them. And you are right, there is a lot of confusing naming conventions that I doubt you agreed to. A deeper dive does show that getAccessibleTdfs and getAssignableTdfs are redundant. They are both trying to solve the same problem, but do it in different ways. getAccessibleTdfs checks if the tdf is in the user's accessed list, getAssignableTdfs checks if the user is either as the tdf's owner or in the tdfs accessor list. I guess the relevant question then is...

-Do you want tdfs still to have owners? Without that, then anyone could delete them in the content manager. -Do you want tdfs still to have accessors or should everyone be able to access/assign each tdf to their classes.

My recommendation is to keep the owners at least for deletion protection.