-
-
Notifications
You must be signed in to change notification settings - Fork 641
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
Allow visitors to create posts with tags #1221
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking really good - apologies for the slow response in looking at this.
app/actions/post.go
Outdated
func (input *CreateNewPost) OnPreExecute(ctx context.Context) error { | ||
input.Tags = make([]*entity.Tag, len(input.TagSlugs)) | ||
for i, slug := range input.TagSlugs { | ||
getTag := &query.GetTagBySlug{Slug: slug} | ||
if err := bus.Dispatch(ctx, getTag); err != nil { | ||
return err | ||
} | ||
|
||
input.Tags[i] = getTag.Result | ||
} | ||
|
||
return nil | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea to make this part of OnPreExecute. You'll just need to update that comment though.
BaseURL string `env:"BASE_URL"` | ||
Locale string `env:"LOCALE,default=en"` | ||
JWTSecret string `env:"JWT_SECRET,required"` | ||
PostCreationWithTagsEnabled bool `env:"POST_CREATION_WITH_TAGS_ENABLED,default=false"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah this looks good. I was contemplating if we should have a nested AppFeatures
struct, and move this into there so that we could add other things in future, but I think we probably don't need to do that...
Looking good. What are you thinking for tests? |
On the action part I'm not sure anything is needed, but tell me if I'm wrong. On the handler part I think the needed tests are :
As of now I think the fails are sent as a 404 error, so I will need to investigate on that before making these tests. |
I suppose it depends - if you're handler tests poke the action then that's great, if they mock the action though then you might need some action tests to check the logic around building up the tags for example - see how you get on 👍 |
Issue: #1211
Visitors can now create post with tags