fix: #704 max-depth not optional.

[CypherpunkSamurai]

Fix #704

max-depth not working as optional as mentioned in docs.

Related issue(s)

#704

Checklist

• I have read the contributing guidelines.
• I am following the
  contributing code guidelines.
• I have read the security policy.
• I confirm that this pull request does not address a security
  vulnerability. If this pull request addresses a security. vulnerability, I
  confirm that I got green light (please contact
  [email protected]) from the maintainers to push
  the changes.
• I have added tests that prove my fix is effective or that my feature
  works.
• I have added or changed the documentation.

Further Comments

This patch fixes issue #704 where max-depth parameter appears to be causing http errors (HTTP 400).

This patch reads the query parameter max-depth into variable d_ and checks if query parameter was not provided before throwing http error.

I hope you will review my PR soon, and accept it for Hacktoberfest. Happy Hacktoberfest :)

[CLAassistant]

All committers have signed the CLA.

[zepatrik]

Further, please add a test that checks whether the right code is returned.
To actually make the parameter required, please add a // required: true comment here:

keto/internal/expand/handler.go

Lines 55 to 56 in 4e65c14

	// in:query
	MaxDepth int `json:"max-depth"`

You will find other definitions that do that, so please look up the right way to do this.
The parameter is not marked as required in the docs (anymore?): https://www.ory.sh/keto/docs/reference/rest-api/#operation/getExpand

[CypherpunkSamurai]

Yes, sorry missed that. Thinking about it a second time, it probably makes more sense to make it actually required as I can't come up with a sane default value that makes sense for most use-cases.

As you're planning on making it required I'll make necessary changes as you suggested. This would make things more clearer :)

Further, please add a test that checks whether the right code is returned.

Ok. I'm new to test driven development area. I will try.

Also, side note, should I change the documentation files too to reflect the changes?

[zepatrik]

Looking pretty good already 👍
Did you find any docs pages that need to be updated except for the API reference? That one is automatically rendered from the openapi schema, which is autogenerated from the code and comments. So should be enough to just adjust the annotations.

[CypherpunkSamurai]

I found a new problem after adding the patch to check max-depth. Is the function supposed to use max-depth as a *int64 rather than a int64?

Update:

Editing out pointerx.int64 seems to have fixed it. Is this ok?

[CypherpunkSamurai]

Hm.

I'll squash those commits into one single commit to not mess up your codebase :)

[CypherpunkSamurai]

I've rebased into one single commit. Tests should work now :)

I'm sorry for turning the git commits. I'm new to PRs thank you for your patient with me :)

[CypherpunkSamurai]

Ok. All checks seems to have passed. Thank You for being patient with me. I'm new to PRs.

I'm still learning please excuse my commits. I'm very sorry for taking so much time.

I had a question and I feel like you're a great developer who can answer. How does one test their code offline before making PR? Everytime I push I see the CI takes over and runs the checks for me. Is it possible to run tests offline?

[zepatrik]

Don't worry, I will squash-merge anyways 😉

Sure, you can run tests locally. We even have a section about that in the README 😂 https://github.com/ory/keto#running-tests
Long story short, the CI basically runns go test -tags sqlite ./... and some other checks like golangci-lint run.

[zepatrik]

Looks perfect now, thank you 🎉