-
Notifications
You must be signed in to change notification settings - Fork 4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changing JSON data based on issue 29 and 30 #34
Conversation
3415bdb
to
41612a7
Compare
|
||
@NotNull | ||
@Valid | ||
public Environment environment; | ||
public String publicTag; |
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.
No longer true, we should get access to Gitlab.cee (as in the tag doesn't have to only public)
|
||
@NotNull | ||
public String revision; | ||
public String version; |
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.
Version of what? :)
|
||
public static String getJsonStreamName(String streamFromFrontend) { | ||
for (Stream stream : Stream.values()) { | ||
if (stream.frontEnd.equals(streamFromFrontend) || stream.backEnd.equals(streamFromFrontend)) { |
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.
In the end, the FE should send only BE data, meaning that we will change this later to just stream.backEnd.equals(streamFromFrontend)
, but let's keep this for now
.contentType(MediaType.APPLICATION_JSON) | ||
.when().post("/build-trigger/trigger") | ||
.then() | ||
.statusCode(500); |
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.
Now this got me thinking - in case we receive a stream that is not in the list, the correct code we should be returning is not 500, but 422
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 think so, too.
41612a7
to
0b601d5
Compare
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.
LGTM
resolves #29