Allow visitors to create posts with tags #1221

MercierMateo · 2024-11-18T00:00:43Z

Issue: #1211

Visitors can now create post with tags

mattwoberts

Looking really good - apologies for the slow response in looking at this.

mattwoberts · 2024-11-23T19:49:08Z

app/actions/post.go

+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
 }


Good idea to make this part of OnPreExecute. You'll just need to update that comment though.

mattwoberts · 2024-11-28T20:00:32Z

app/pkg/env/env.go

+	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"`


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...

mattwoberts · 2024-11-28T20:01:58Z

Looking good. What are you thinking for tests?

MercierMateo · 2024-12-01T21:34:16Z

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 :

create a post with an inexistent tag -> should fail as bad request
create a post with a private tag as a standard user -> should fail as bad request
create a post with a public tag as a standard user -> should succeed
create a post with a public tag and a private tag as an administrator -> should succeed

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.

mattwoberts · 2024-12-02T08:32:14Z

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 :

create a post with an inexistent tag -> should fail as bad request

create a post with a private tag as a standard user -> should fail as bad request

create a post with a public tag as a standard user -> should succeed

create a post with a public tag and a private tag as an administrator -> should succeed

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 👍

…http 400 status code

MercierMateo · 2024-12-10T23:39:55Z

I created the tests, the actions are poked and function well.
Are these tests OK and enough for the backend part ?

mattwoberts

Cheers, yeah this is coming on nicely, there might be more tests as it progresses but no issues here

app/handlers/apiv1/post_test.go

Co-authored-by: Matt Roberts <[email protected]>

MercierMateo · 2024-12-16T23:08:09Z

Thanks, i will look into the UI further but I struggle a bit more with this part.

I see there are errors on the ui test side, is it normal ? I don't think I made changes on the UI that could trigger these errors but I'm not sure.

mattwoberts · 2024-12-17T08:18:04Z

Thanks, i will look into the UI further but I struggle a bit more with this part.

I see there are errors on the ui test side, is it normal ? I don't think I made changes on the UI that could trigger these errors but I'm not sure.

Yeah it looks like something has changed, I think a new playwright version is causing problems for our github actions (microsoft/playwright#30368) - it's nothing you've done. I'll take a look at this

mattwoberts · 2024-12-17T08:31:16Z

@MercierMateo Ugh, something went badly wrong in github - I tried to add a commit to your PR and ended up merging it into main - sorry about this. I'll sort it out

mattwoberts · 2024-12-17T11:53:47Z

@MercierMateo Ok confession time. I was trying out the gh command line tool for the first time, I made a change to your pull request, and thought I'd pushed it as a commit onto your fork. Turns out I had messed up badly, and pushed it to main, which resulted in your PR being merged into main and closed. I'd asked chatGPT how to do this and just blindly copied the command without properly checking it. Never trust ChatGPT.

What an idiot I am.

Sorry.

I see 2 options here

I revert all your commits from main to get it back to the state it was in before I did this. However I'm not sure what that would do for your pull request, and how we'd get the commits back into a pull request.
We just accept what I've done, and call that "phase 1" of your work. The functionality is working, you can create posts with tags, it's just that there is no way to do that from the UI. So we can get main working by fixing the issues with the build, and then you can create another pull request, for "phase 2" that then is the work to add this to the UI.

What are your thoughts on this, and do you have any other suggestions?

Thanks!

Matt

MercierMateo · 2024-12-17T12:25:05Z

Ok no problem, I'll add this to my list of reasons to avoid using ChatGPT

I think option 2 is fine. As I separated my work on the UI from my main branch, only the backend part was merged on your main. I can easily create a new PR and continue working on the UI part of the feature.

Obviously if there are any required fixes on the backend part we can go for option 1, but it doesn't seem like it.

mattwoberts · 2024-12-17T20:34:50Z

Yes I think this is the best option - it's much easier all round I think, I've just got to sort out the issues causing the build to break on my side, and you can work on the UI.

mattwoberts · 2024-12-18T22:01:06Z

Great stuff. I've fixed the issue with the tests failing now. Matt

…

On Tue, Dec 17, 2024 at 12:25 PM MercierMateo ***@***.***> wrote: Ok no problem, I'll add this to my list of reasons to avoid using ChatGPT I think option 2 is fine. As I separated my work on the UI from my main branch, only the backend part was merged on your main. I can easily create a new PR and continue working on the UI part of the feature. Obviously if there are any required fixes on the backend part we can go for option 1, but it doesn't seem like it. — Reply to this email directly, view it on GitHub <#1221 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA2VDFTOX7QE32E7M5YZNL2GAJ3NAVCNFSM6AAAAABR6O4GICVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNBYGMZDEOBYG4> . You are receiving this because you modified the open/close state.Message ID: ***@***.***>

Mercier Mateo and others added 3 commits October 6, 2024 20:22

Feature add tags on post creation first version

0988f47

Merge branch 'getfider:main' into main

def457f

Add authorization on post creation with tags

dddd043

MercierMateo mentioned this pull request Nov 18, 2024

Allow visitors to create posts with tags #1211

Open

mattwoberts approved these changes Nov 23, 2024

View reviewed changes

Mercier Mateo added 2 commits November 26, 2024 11:03

Fix comment on CreateNewPost action

782dcaf

Add feature flag on post creation with tags

3b44094

mattwoberts reviewed Nov 28, 2024

View reviewed changes

Mercier Mateo added 2 commits December 11, 2024 00:37

Move tag not found error handling to validate in order to get proper …

7d4c70a

…http 400 status code

Add tests for post creation with tags

1a4712c

Merge branch 'main' into main

f010b9f

mattwoberts approved these changes Dec 16, 2024

View reviewed changes

app/handlers/apiv1/post_test.go Outdated Show resolved Hide resolved

Fix grammar on test

513d517

Co-authored-by: Matt Roberts <[email protected]>

mattwoberts merged commit 513d517 into getfider:main Dec 17, 2024
4 of 8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow visitors to create posts with tags #1221

Allow visitors to create posts with tags #1221

MercierMateo commented Nov 18, 2024

mattwoberts left a comment

mattwoberts Nov 23, 2024

mattwoberts Nov 28, 2024

mattwoberts commented Nov 28, 2024

MercierMateo commented Dec 1, 2024

mattwoberts commented Dec 2, 2024

MercierMateo commented Dec 10, 2024

mattwoberts left a comment

MercierMateo commented Dec 16, 2024

mattwoberts commented Dec 17, 2024

mattwoberts commented Dec 17, 2024

mattwoberts commented Dec 17, 2024

MercierMateo commented Dec 17, 2024

mattwoberts commented Dec 17, 2024

mattwoberts commented Dec 18, 2024 via email

Allow visitors to create posts with tags #1221

Allow visitors to create posts with tags #1221

Conversation

MercierMateo commented Nov 18, 2024

mattwoberts left a comment

Choose a reason for hiding this comment

mattwoberts Nov 23, 2024

Choose a reason for hiding this comment

mattwoberts Nov 28, 2024

Choose a reason for hiding this comment

mattwoberts commented Nov 28, 2024

MercierMateo commented Dec 1, 2024

mattwoberts commented Dec 2, 2024

MercierMateo commented Dec 10, 2024

mattwoberts left a comment

Choose a reason for hiding this comment

MercierMateo commented Dec 16, 2024

mattwoberts commented Dec 17, 2024

mattwoberts commented Dec 17, 2024

mattwoberts commented Dec 17, 2024

MercierMateo commented Dec 17, 2024

mattwoberts commented Dec 17, 2024

mattwoberts commented Dec 18, 2024 via email