Enable Admire Token pt1 #1160
Enable Admire Token pt1 #1160
Conversation
Rohan-cp
requested review from
benny-conn,
radazen and
jarrel-b
as code owners
| @@ -0,0 +1 @@ | |||
| ALTER TABLE admires ADD COLUMN IF NOT EXISTS token_id varchar(255) references token(id); | |||
There was a problem hiding this comment.
I realized we'll also want to add a constraint on this table so that you can't admire the same token twice. You can add a line to this migration that goes something like this:
create unique index admire_actor_id_token_id_idx on admires(actor_id, token_id) where deleted = false;
| @@ -52,6 +52,28 @@ func (a *AdmireRepository) CreateAdmire(ctx context.Context, feedEventID, postID | |||
|
|
|||
| } | |||
|
|
|||
| func (a *AdmireRepository) CreateAdmireToken(ctx context.Context, tokenID, actorID persist.DBID) (persist.DBID, error) { | |||
| var tokenString sql.NullString | |||
There was a problem hiding this comment.
fyi: there's a helper in util/helpers.go named ToNullString which could be useful here!
There was a problem hiding this comment.
hmm, not sure how I would use it here because I would still need to check if tokenID != "" before doing tokenID.String() (to then pass to ToNullString) right?
| func (api InteractionAPI) AdmireToken(ctx context.Context, tokenID persist.DBID) (persist.DBID, error) { | ||
| // Validate | ||
| if err := validate.ValidateFields(api.validator, validate.ValidationMap{ | ||
| "tokenID": validate.WithTag(tokenID, "required"), |
There was a problem hiding this comment.
I think it would be great to check that not only the tokenID is provided, but also if a token with that tokenID exists
graphql/schema/schema.graphql
Outdated
| admire: Admire @goField(forceResolver: true) | ||
| } | ||
|
|
||
| union AdmireTokenPayloadOrError = AdmireTokenPayload | ErrInvalidInput | ErrNotAuthorized |
There was a problem hiding this comment.
If we add the check for if the token exists, can we add ErrTokenNotFound as a possible returned type here?
graphql/graphql_test.go
Outdated
| userF := newUserFixture(t) | ||
| c := authedHandlerClient(t, userF.ID) | ||
| alice := newUserWithTokensFixture(t) | ||
| admireToken(t, ctx, c, alice.TokenIDs[0]) |
There was a problem hiding this comment.
not sure how to test further if admireToken is working because it doesn't return anything
There was a problem hiding this comment.
This seems fine to me! One thing we can test is to return the tokenID from admireToken and verify that it's the token we expect.
We can also test admiring the same token twice to verify that you can only admire a token once per user.
graphql/graphql_test.go
Outdated
| userF := newUserFixture(t) | ||
| c := authedHandlerClient(t, userF.ID) | ||
| alice := newUserWithTokensFixture(t) | ||
| admireToken(t, ctx, c, alice.TokenIDs[0]) |
There was a problem hiding this comment.
This seems fine to me! One thing we can test is to return the tokenID from admireToken and verify that it's the token we expect.
We can also test admiring the same token twice to verify that you can only admire a token once per user.
publicapi/interaction.go
Outdated
| return "", err | ||
| } | ||
|
|
||
| /* |
There was a problem hiding this comment.
I think we can remove this comment since the table constraint we added should enforce this now.
| @@ -52,6 +52,28 @@ func (a *AdmireRepository) CreateAdmire(ctx context.Context, feedEventID, postID | |||
|
|
|||
| } | |||
|
|
|||
| func (a *AdmireRepository) CreateAdmireToken(ctx context.Context, tokenID, actorID persist.DBID) (persist.DBID, error) { | |||
| var tokenString sql.NullString | |||
Testing:
wrote the
testAdmireTokentest