-
Notifications
You must be signed in to change notification settings - Fork 0
Create org #8
Create org #8
Changes from 7 commits
cca97b6
0c63210
0003931
ce47c90
7cd5d91
fee4d1c
572c6dc
d1b0f45
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check for status code here instead of checking if the body exists as a 204 status code response has an empty body. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 I'll add a comment in the code There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed. I didn't know what to return. Please use your judgement. 👍 |
||
|
||
def parse_body_response({:ok, response}) do | ||
# Logger.debug(response) # very noisy! | ||
body = Map.get(response, :body) | ||
|
||
if body == nil || byte_size(body) == 0 do | ||
Logger.warning("GiteaHttp.parse_body_response: response body is nil!") | ||
{:error, :no_body} | ||
|
@@ -63,6 +64,7 @@ defmodule Gitea.Http do | |
@spec get(String.t()) :: {:ok, map} | {:error, any} | ||
def get(url) do | ||
Logger.debug("GiteaHttp.get #{url}") | ||
|
||
inject_poison().get(url, json_headers()) | ||
|> parse_body_response() | ||
end | ||
|
@@ -94,8 +96,9 @@ defmodule Gitea.Http do | |
# Logger.debug("raw_markdown: #{raw_markdown}") | ||
headers = [ | ||
{"Accept", "text/html"}, | ||
auth_header(), | ||
auth_header() | ||
] | ||
|
||
inject_poison().post(url, raw_markdown, headers) | ||
end | ||
|
||
|
@@ -109,6 +112,7 @@ defmodule Gitea.Http do | |
def post(url, params \\ %{}) do | ||
Logger.debug("GiteaHttp.post #{url}") | ||
body = Jason.encode!(params) | ||
|
||
inject_poison().post(url, body, json_headers()) | ||
|> parse_body_response() | ||
end | ||
|
@@ -120,6 +124,7 @@ defmodule Gitea.Http do | |
@spec delete(String.t()) :: {:ok, map} | {:error, any} | ||
def delete(url) do | ||
Logger.debug("GiteaHttp.delete #{url}") | ||
|
||
inject_poison().delete(url <> "?token=#{access_token()}") | ||
|> parse_body_response() | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,16 +102,24 @@ defmodule Gitea.HTTPoisonMock do | |
def post(url, body, headers) do | ||
Logger.debug("Gitea.HTTPoisonMock.post/3 #{url}") | ||
|
||
if String.contains?(url, "markdown/raw") do | ||
post_raw_html(url, body, headers) | ||
else | ||
body_map = Jason.decode!(body) |> Useful.atomize_map_keys() | ||
cond do | ||
url =~ "markdown/raw" -> | ||
post_raw_html(url, body, headers) | ||
|
||
response_body = | ||
make_repo_create_post_response_body(body_map.name) | ||
|> Jason.encode!() | ||
url =~ "/repo" -> | ||
body_map = Jason.decode!(body) |> Useful.atomize_map_keys() | ||
|
||
{:ok, %HTTPoison.Response{body: response_body, status_code: 200}} | ||
response_body = | ||
make_repo_create_post_response_body(body_map.name) | ||
|> Jason.encode!() | ||
|
||
{:ok, %HTTPoison.Response{body: response_body, status_code: 200}} | ||
|
||
url =~ "/orgs" -> | ||
{:ok, %HTTPoison.Response{body: Jason.encode!(%{username: "new_org"}), status_code: 200}} | ||
|
||
true -> | ||
{:ok, %HTTPoison.Response{body: Jason.encode!(""), status_code: 404}} | ||
end | ||
end | ||
|
||
|
@@ -135,11 +143,22 @@ defmodule Gitea.HTTPoisonMock do | |
""" | ||
def delete(url) do | ||
Logger.debug("Gitea.HTTPoisonMock.delete/1 #{url}") | ||
# check delete request endpooints | ||
cond do | ||
# match delete org | ||
url =~ "/orgs" -> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Check what is the delete request (delete org or delete repo at the moment) and returns response accordingly |
||
{:ok, | ||
%HTTPoison.Response{ | ||
body: "", | ||
status_code: 204 | ||
}} | ||
|
||
{:ok, | ||
%HTTPoison.Response{ | ||
body: Jason.encode!(%{deleted: List.first(String.split(url, "?"))}), | ||
status_code: 200 | ||
}} | ||
true -> | ||
{:ok, | ||
%HTTPoison.Response{ | ||
body: Jason.encode!(%{deleted: List.first(String.split(url, "?"))}), | ||
status_code: 200 | ||
}} | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
any reason we don't just pass in a
Map
ofparams
instead of theorg_name
andopts
? 💭There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw the
remote_org_create
function having only one required argument (theorg_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 argumentsee: https://elixir-lang.org/getting-started/keywords-and-maps.html#keyword-lists
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, sadly,
remote_org_create/1
inGogs
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 (onlyparams.username
is required) will make it immediately clear to the person using the app that theirparams
are passed in directly.