Create org #8
Create org #8
Conversation
Create a new organisation on Gitea
Add case to mock the different type of post requests
Codecov Report
@@ Coverage Diff @@
## main #8 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 5 5
Lines 115 128 +13
=========================================
+ Hits 115 128 +13
Continue to review full report at Codecov.
|
We are using mock post request to creat repos, orgs and to convert html/markdow. For any other endpoints the mock returns 404 status code
Add test for delete organisation
Update test to remove unused body value
| @@ -40,10 +40,11 @@ defmodule Gitea.Http do | |||
| """ | |||
| @spec parse_body_response({atom, String.t()} | {:error, any}) :: {:ok, map} | {:error, any} | |||
| def parse_body_response({:error, err}), do: {:error, err} | |||
| def parse_body_response({:ok, response = %{status_code: 204}}), do: {:ok, response} | |||
There was a problem hiding this comment.
Check for status code here instead of checking if the body exists as a 204 status code response has an empty body.
see https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/204 and https://gitea-server.fly.dev/api/swagger#/organization/orgDelete
There was a problem hiding this comment.
Worth including this comment in-line (in the code) as it will be lost here in the PR ...
There was a problem hiding this comment.
The reason I've added this definition is to avoid changing the code below:
if body == nil || byte_size(body) == 0 do
My first thought was to remove the byte_size(body) == 0 condition as I don't think parse_body_response should return {:error, :no_body} when the body is an empty string (it is defined like this when deleting orgs or repos). However I didn't want to break existing tests in case I missed something.
I'll add a comment in the code
There was a problem hiding this comment.
Agreed. I didn't know what to return. Please use your judgement. 👍
| # check delete request endpooints | ||
| cond do | ||
| # match delete org | ||
| url =~ "/orgs" -> |
There was a problem hiding this comment.
Check what is the delete request (delete org or delete repo at the moment) and returns response accordingly
when creating a new org we can pass a keyword list options
SimonLab
added
the
awaiting-review
lib/gitea.ex
Outdated
| and define the description, full_name and visibility | ||
| """ | ||
| @spec remote_org_create(String.t(), list()) :: {:ok, map} | {:error, any} | ||
| def remote_org_create(org_name, opts \\ []) do |
There was a problem hiding this comment.
any reason we don't just pass in a Map of params instead of the org_name and opts ? 💭
There was a problem hiding this comment.
I saw the remote_org_create function having only one required argument (the org_name) and the other values (description, full_name and visibility) as optional similar to the gitea api and the UI. In this case I think it makes sense to use a keyword list as it is used for this type of optional argument
see: https://elixir-lang.org/getting-started/keywords-and-maps.html#keyword-lists
Keyword lists are a data-structure used to pass options to functions
However if you consider that all the values are required we can pass a Map (or even define a struct).
Let me know if you prefer the "map" version of the function
There was a problem hiding this comment.
yeah, sadly, remote_org_create/1 in Gogs didn't work dwyl/gogs#8 (comment) so I didn't pursue it ...
I find keyword lists cumbersome.
in this case having a single params argument with all the necessary fields (only params.username is required) will make it immediately clear to the person using the app that their params are passed in directly.
nelsonic
added
question
Use a Map instead of keyword list for optional API parameters see: #8 (comment)
| @@ -40,10 +40,11 @@ defmodule Gitea.Http do | |||
| """ | |||
| @spec parse_body_response({atom, String.t()} | {:error, any}) :: {:ok, map} | {:error, any} | |||
| def parse_body_response({:error, err}), do: {:error, err} | |||
| def parse_body_response({:ok, response = %{status_code: 204}}), do: {:ok, response} | |||
There was a problem hiding this comment.
Agreed. I didn't know what to return. Please use your judgement. 👍
ref: #7