globe-and-citizen / layer8

This repo contains the Layer8 Resource/Authentication Server, Proxy, and Service Provider Mocks
GNU General Public License v2.0
1 stars 2 forks source link

buildResponse Funciton Update #118

Open stravid87 opened 3 months ago

stravid87 commented 3 months ago

We need to update the function func BuildResponse(status bool, message string, data interface{}) Response { on line 66 of server/resource_server/utils.go to get away from the "OK!" message.

Unfortunately, checking for the "OK!" message has made it's way into the code in multiple places (See "profile.html" and "register.html")

To fix this issue, I think that BuildResponse should probably receive a status code as an int instead of a bool and return this to the frontend?

That is, it should take in the w http.ResponseWriter and add the status code to the headers (not include it in the body).

In this way, the user could check "response.headers.status" from the headers instead of "response.body.message" after parsing.

Acceptance Criteria

1) Refactor the function so that it adds the status code dynamically (e.g., 200, 201, 20x, etc) 2) Unit tests that "fail" should no longer have status 200 and should pass

Pull Requests

PR 128 in Layer8 PR: https://github.com/globe-and-citizen/layer8/pull/128

stravid87 commented 3 months ago

Examples: authentication_test.go that function:

func Test_PostLoginHandler_InvalidCredentials_OK(t *testing.T) { func Test_PostLoginHandler_TokenNotExists_OK(t *testing.T) {

The goal could be to improve on this so that these two tests would return the appropriate status codes

stravid87 commented 3 months ago

Looking again at this issue, I see that maybe I am confused. Following this func Test_PostLoginHandler_InvalidCredentials_OK(t *testing.T) { from the testing suite, of authentication_test.go I see that the status code actually comes from the parseHTML function that is injected on line 153 of main.go. Therefore, refactoring "buildResponse" will not solve this problem.

stravid87 commented 3 months ago

In hindsight, I was in fact confused. After buildResponse() is invoked, the caller can simply invoke w.WriteHeader(...) before the response.writer is triggered for examply, by using json.NewEncoder())

stravid87 commented 2 months ago

Nice work.

@ternakkode can you validate / review so that @huzaifamk can review?

huzaifamk commented 2 months ago

PR: https://github.com/globe-and-citizen/layer8/pull/128

stravid87 commented 2 months ago

July 29

Double check that the refactor of getLoginHandler hasn't broken functionality (now that you've got the tests working).