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

backend changes for post read status sync #212

Merged
merged 11 commits into from
Dec 21, 2021
Merged

Conversation

gopricy
Copy link
Collaborator

@gopricy gopricy commented Dec 16, 2021

No description provided.

utils/redis_utils.go Outdated Show resolved Hide resolved
utils/redis_utils.go Outdated Show resolved Hide resolved
DB *gorm.DB
SignalChans *SignalChannels
DB *gorm.DB
RedisClient *utils.RedisClient
Copy link
Owner

Choose a reason for hiding this comment

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

Is this initialized anywhere?

postsId: [String!]!
}

input GetPostsReadStatusInput {
Copy link
Owner

Choose a reason for hiding this comment

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

How does user call GetPostsReadStatus?

@@ -166,6 +177,8 @@ type Mutation {
addWeiboSubSource(input: AddWeiboSubSourceInput!): SubSource

syncUp(input: SeedStateInput): SeedState

markPostsAsRead(input: MarkPostsAsReadInput!): Boolean!
Copy link
Owner

Choose a reason for hiding this comment

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

You don't need to use Boolean! as return value. Error means incorrect

Copy link
Collaborator Author

@gopricy gopricy Dec 18, 2021

Choose a reason for hiding this comment

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

ardatan/graphql-tools#277
I tried to return nothing but seems like graphql doesn't allow it so a boolean was a work around

@@ -3,9 +3,11 @@ enum SignalType {
# Instruct client side to pull seed state. This is the first signal send to
# client side application.
SEED_STATE
READ_POST
Copy link
Owner

Choose a reason for hiding this comment

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

POST_READ_STATUS

server/graph/signal.graphqls Show resolved Hide resolved
)

func (r *signalResolver) SignalPayload(ctx context.Context, obj *model.Signal) (*string, error) {
panic(fmt.Errorf("not implemented"))
Copy link
Owner

Choose a reason for hiding this comment

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

?

You need to copy from obj to output string, otherwise you won;t access it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

since we need set unread functionality, I will change it from string to a struct


var ctx = context.Background()

func GetRedisClient() *RedisClient {
Copy link
Owner

Choose a reason for hiding this comment

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

I don't see this function accessed anywhere?

Comment on lines 351 to 389
err := r.RedisClient.MarkPostsAsRead(input.PostsID, input.UserID)
if err != nil {
return false, err
}
return true, nil
Copy link
Owner

Choose a reason for hiding this comment

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

Where do you push signal+payload to all end user sessions?

@@ -455,3 +467,13 @@ func (r *Resolver) Subscription() generated.SubscriptionResolver { return &subsc
type mutationResolver struct{ *Resolver }
type queryResolver struct{ *Resolver }
type subscriptionResolver struct{ *Resolver }

// !!! WARNING !!!
Copy link
Collaborator

Choose a reason for hiding this comment

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

remove this unused code

utils/redis_utils.go Outdated Show resolved Hide resolved
"github.com/go-redis/redis/v8"
)

type RedisClient struct {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This package is more like a "ReadStatusSotre" instead of Redis Client, maybe we can pass in a real redis client as a dependency, and keep only the business logic in this struct.

})}
}

func PostKey(userId string, postId string) string {
Copy link
Collaborator

Choose a reason for hiding this comment

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

wrap this in a struct and have a ser/deser API so we can extend and reuse in the future.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

utils/redis_utils.go Outdated Show resolved Hide resolved
@realcwl
Copy link
Owner

realcwl commented Dec 20, 2021

fix test error?

server/handlers.go Outdated Show resolved Hide resolved
model/post.go Outdated
@@ -97,4 +97,5 @@ type Post struct {
DeduplicateId string `json:"deduplicate_id"`
SemanticHashing string `json:"semantic_hashing"`
Tag string `json:"tag"`
IsRead bool `json:"is_read" gorm:"-"`
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is "-"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@gopricy gopricy merged commit 0a568a1 into master Dec 21, 2021
@gopricy gopricy deleted the sync-post-read-status branch December 21, 2021 06:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants