source-academy / stories-backend

Backend of Source Academy extension for Stories support.
0 stars 0 forks source link

Add group filter to stories #98

Closed YaleChen299 closed 1 year ago

YaleChen299 commented 1 year ago

Missing Permission check. Current behaviour of the model function is:

In the controller, the current routing and middle ware guarantees that the list all route will have a valid groupID.

Part of #15.

github-actions[bot] commented 1 year ago

Coverage Status

coverage: 49.36% (-1.3%) from 50.642% when pulling 2741bea04f930640b9202fea1c5b8d4d96770f78 on add_group_filter_to_stories into ba486fbffe8f00782bea3628325e8197674a847e on main.

RichDom2185 commented 1 year ago

Blocked by #97 (depends on the same migration file).

RichDom2185 commented 1 year ago

Direction looks good. Just one question: does setting to nil actually filter WHERE group_id IS NULL? Or does it simply ignore the field, can we test this behaviour?

I'm asking because when setting values, we need to explicitly use an NULL expression in gorm (nil fields are simply ignored, e.g. see #85).

YaleChen299 commented 1 year ago

Direction looks good. Just one question: does setting to nil actually filter WHERE group_id IS NULL? Or does it simply ignore the field, can we test this behaviour?

I'm asking because when setting values, we need to explicitly use an NULL expression in gorm (nil fields are simply ignored, e.g. see #85).

I think i have tests for that, when i use the struct where, with group id as null, it will explicit find those stories without groups.

Actually, let me refine the tests. I might not cover the case well.

Edit: passing nil will get all the stories.

RichDom2185 commented 1 year ago

Edit: passing nil will get all the stories.

Ok, I will update the PR description.