Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add stories moderation #120

Draft
wants to merge 20 commits into
base: main
Choose a base branch
from
Draft

Add stories moderation #120

wants to merge 20 commits into from

Conversation

keyansheng
Copy link

@keyansheng keyansheng commented Mar 20, 2024

Kelly Wong and others added 7 commits March 20, 2024 22:43
Running pre-commit hook...
Go code is not formatted, commit blocked!
The following files are not formatted:

./controller/stories/update.go
./internal/auth/middleware.go
./internal/permissions/users/permissions.go
./internal/permissions/users/role.go
./internal/router/router.go
./model/stories.go
./model/usergroups.go
./params/stories/moderate.go

Run 'make format' to format your files and try again.
@RichDom2185
Copy link
Member

image

Can you run go get and commit the changes to fix the CI failure? Thanks!

@coveralls
Copy link

coveralls commented Mar 26, 2024

Coverage Status

coverage: 47.69% (-2.0%) from 49.653%
when pulling 54d9a21 on stories-moderation
into 7288818 on main.

@keyansheng keyansheng added the enhancement New feature or request label Apr 12, 2024
@keyansheng keyansheng changed the title Stories Moderation draft Add stories moderation Apr 12, 2024
Copy link
Member

@RichDom2185 RichDom2185 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, thanks for working on this! I have some preliminary comments below:

Comment on lines +12 to +17
const (
Draft StoryStatus = iota
Pending
Rejected
Published
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This enum should not be part of the model package and instead be a separate stories enum package.

Comment on lines +50 to +113
func GetAllPublishedStories(db *gorm.DB, groupID *uint) ([]Story, error) {
var stories []Story
err := db.
Where("status = ?", int(Published)).
Where("group_id = ?", groupID).
Preload(clause.Associations).
// TODO: Abstract out the sorting logic
Order("pin_order ASC NULLS LAST, title ASC, content ASC").
Find(&stories).
Error
if err != nil {
return stories, database.HandleDBError(err, "story")
}
return stories, nil
}

func GetAllPendingStories(db *gorm.DB, groupID *uint) ([]Story, error) {
var stories []Story
err := db.
Where("status = ?", int(Pending)).
Where("group_id = ?", groupID).
Preload(clause.Associations).
// TODO: Abstract out the sorting logic
Order("pin_order ASC NULLS LAST, title ASC, content ASC").
Find(&stories).
Error
if err != nil {
return stories, database.HandleDBError(err, "story")
}
return stories, nil
}

func GetAllStoriesByStatus(db *gorm.DB, groupID *uint, status StoryStatus) ([]Story, error) {
var stories []Story
err := db.
Where("status = ?", int(status)).
Where("group_id = ?", groupID).
Preload(clause.Associations).
// TODO: Abstract out the sorting logic
Order("pin_order ASC NULLS LAST, title ASC, content ASC").
Find(&stories).
Error
if err != nil {
return stories, database.HandleDBError(err, "story")
}
return stories, nil
}

func GetAllAuthorStoriesByStatus(db *gorm.DB, groupID *uint, userID *int, status StoryStatus) ([]Story, error) {
var stories []Story
err := db.
Where("status = ?", int(status)).
Where("group_id = ?", groupID).
Where("author_id = ?", userID).
Preload(clause.Associations).
// TODO: Abstract out the sorting logic
Order("pin_order ASC NULLS LAST, title ASC, content ASC").
Find(&stories).
Error
if err != nil {
return stories, database.HandleDBError(err, "story")
}
return stories, nil
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extensive code duplication. This can be easily combined.

Content string `json:"content"`
PinOrder *int `json:"pinOrder"`
Status int `json:"status"`
StatusMessage string `json:"statusMessage"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StatusMessage has type string here, but in the model, it is of the type *string.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which one is the correct one?

-- +migrate Up

ALTER TABLE stories
ADD COLUMN status INT;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any constraints we want to add?

-- +migrate Up

ALTER TABLE stories
ADD COLUMN status_message TEXT;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any constraints we want to add?

Comment on lines +58 to +61
r.Get("/draft", handleAPIError(stories.HandleListDraft))
r.Get("/pending", handleAPIError(stories.HandleListPending))
r.Get("/published", handleAPIError(stories.HandleListPublished))
r.Get("/rejected", handleAPIError(stories.HandleListRejected))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The filtering should be done via a query parameter, not separate routes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

4 participants