Skip to content
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.

Sign up for GitHub

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

Added the required status code StatusCodes.CREATED for the createRele… #231

Merged
merged 7 commits into from
Sep 15, 2020
Merged

Added the required status code StatusCodes.CREATED for the createRele… #231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
34439bf
Added the required status code StatusCodes.CREATED for the createRele…
bsutton Aug 13, 2020
5d41ad0
null bool was returned when errors was null which was causing an asse…
bsutton Aug 13, 2020
5700cda
fixed formatting.
bsutton Aug 13, 2020
7ba7629
Added log to handle a 404 which occurs if the tag doesn't exist.
bsutton Aug 24, 2020
16bfe55
Added asserts to ensure that each argument to RepositorySlug is non-null
bsutton Aug 24, 2020
dfe06d7
Added new Exception for ReleaseNotFound.
bsutton Aug 24, 2020
6caf3ed
Modified getReleaseByTagName to thrown an exception if the release ca…
bsutton Sep 15, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion lib/src/common/model/repos.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,10 @@ class RepositorySlug {
/// Repository Name
String name;

RepositorySlug(this.owner, this.name);
RepositorySlug(this.owner, this.name) {
ArgumentError.checkNotNull(owner, 'owner');
ArgumentError.checkNotNull(name, 'name');
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


/// Creates a Repository Slug from a full name.
factory RepositorySlug.full(String f) {
Expand Down
2 changes: 1 addition & 1 deletion lib/src/common/model/repos_releases.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ class Release {
String getUploadUrlFor(String name, [String label]) =>
"${uploadUrl.substring(0, uploadUrl.indexOf('{'))}?name=$name${label != null ? ",$label" : ""}";

bool get hasErrors => errors?.isNotEmpty;
bool get hasErrors => errors == null ? false : errors.isNotEmpty;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a feeling that returning null from hasErrors was causing a problem which is why I changed it to return false. I can't quite remember the circumstances.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just checked and the method createRelease has the following line:

if (release.hasErrors) {

As such hasErrors should never be null.
My experience with dart is that passing a bool with a null value always ends with someone crying.

}

/// Model class for a release asset.
Expand Down
27 changes: 20 additions & 7 deletions lib/src/common/repos_service.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -968,10 +968,23 @@ class RepositoriesService extends Service {

/// Fetches a single release by the release tag name.
///
/// Throws a [ReleaseNotFound] exception if the release
/// doesn't exist.
///
/// API docs: https://developer.github.com/v3/repos/releases/#get-a-release-by-tag-name
Future<Release> getReleaseByTagName(RepositorySlug slug, String tagName) =>
github.getJSON('/repos/${slug.fullName}/releases/tags/$tagName',
convert: (i) => Release.fromJson(i));
Future<Release> getReleaseByTagName(
RepositorySlug slug, String tagName) async {
return github.getJSON(
'/repos/${slug.fullName}/releases/tags/$tagName',
convert: (i) => Release.fromJson(i),
statusCode: StatusCodes.OK,
fail: (http.Response response) {
if (response.statusCode == 404) {
throw ReleaseNotFound.fromTagName(github, tagName);
}
},
);
}

/// Creates a Release based on the specified [createRelease].
///
Expand All @@ -986,10 +999,10 @@ class RepositoriesService extends Service {
ArgumentError.checkNotNull(slug);
ArgumentError.checkNotNull(createRelease);
final release = await github.postJSON<Map<String, dynamic>, Release>(
'/repos/${slug.fullName}/releases',
convert: (i) => Release.fromJson(i),
body: GitHubJson.encode(createRelease.toJson()),
);
'/repos/${slug.fullName}/releases',
convert: (i) => Release.fromJson(i),
body: GitHubJson.encode(createRelease.toJson()),
statusCode: StatusCodes.CREATED);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part looks ok to me.

if (release.hasErrors) {
final alreadyExistsErrorCode = release.errors.firstWhere(
(error) => error['code'] == 'already_exists',
Expand Down
6 changes: 6 additions & 0 deletions lib/src/common/util/errors.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ class RepositoryNotFound extends NotFound {
: super(github, 'Repository Not Found: $repo');
}

/// Release not found
class ReleaseNotFound extends NotFound {
const ReleaseNotFound.fromTagName(GitHub github, String tagName)
: super(github, 'Release for tagName $tagName Not Found.');
}

/// GitHub User was not found
class UserNotFound extends NotFound {
const UserNotFound(GitHub github, String user)
Expand Down