154088163 Ft Users can get top rated resources #43
Conversation
- Add handler to get top rated resources - Add tests for getting top rated resources - Add test data to utils_test file
| @@ -154,7 +154,7 @@ type Resource struct { | |||
| Privacy string `sql:",notnull" json:",omitempty"` | |||
| Type string `sql:",notnull" json:",omitempty"` | |||
| Views int64 `json:",omitempty"` | |||
| Recommendations int64 `json:",omitempty"` | |||
| Recommendations int64 `sql:",notnull" json:",omitempty"` | |||
There was a problem hiding this comment.
Can you test the create resource endpoint please. I'm thinking this might be a breaking change for it.
There was a problem hiding this comment.
@tomipaul Are you saying write test for this or run the tests we already have for it ?
There was a problem hiding this comment.
@mikey2020 I don't remember what the implementation of the create resource endpoint looks like. But I'm thinking a not null directive could have broken something. So in the mean time you could just run all the tests we already have. I will take a look at the implementation tonight. You can take a look at it too so we can be sure nothing breaks.
| ) | ||
| return | ||
| } | ||
| fmt.Println(err) |
There was a problem hiding this comment.
No I dont think so
| t.Fatalf("Expected resource title %s to equal %s", actualResourceTitle1, expectedResourceTitle1) | ||
| } | ||
|
|
||
| }) |
There was a problem hiding this comment.
About the tests, you would remember I updated the supertest we were using to be able to use the request to make assertions like this in all scenarios
Request(testServer.URL, t).
Get(fmt.Sprintf("%v%v", resourceURI, publicResource.Id)).
Set("authorization", userToken).
Expect(200).
Expect("Content-Type", "application/json").
Expect(expectedResponse).
End()
We can still merge this and later we'll have a task to harmonize every tests to follow that pattern. The advantage I was trying to get was to make the whole test structure simple and consistent; simulate expected response and just do a whole big assert. I'm not sure but it seems I made it in such a way that it will work for partial responses too.
I will take a look.
|
@mikey2020 I have some comments |
Why is this important?
Description of task to be completed?