Implemented storing and getting data #23
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Implemented getting information
Layla-P
requested changes
Oct 2, 2020
| _logger.LogTrace($"Sending request to Github for pull requests on repositoy: {name}"); | ||
| var prs = await _client.PullRequest.GetAllForRepository(owner, name, new PullRequestRequest() { State = ItemStateFilter.All }); | ||
|
|
||
| return prs.Select(pr => new Models.DTOs.PullRequest(pr.Number, pr.Url)).ToList(); |
There was a problem hiding this comment.
Hey Rick!
Can we return a var listOfPrs for example instead of directly returning the select statement as it makes it easier for less expreienced developers to read and understand, imo.
Same goes for the rest of the methods here.
| _logger.LogTrace($"Sending request to Github for repositories belonging to user: {owner}"); | ||
| var repositories = await _client.Repository.GetAllForUser(owner); | ||
|
|
||
| return repositories.Select(r => new Models.DTOs.Repository(owner, r.Name, r.Url)).ToList(); |
There was a problem hiding this comment.
Is there a namespace conflict or could we move Models.DTOs to a using statement?
Comment on lines
56
to
+83
| var info = entity.RowKey.Split(':', StringSplitOptions.RemoveEmptyEntries); | ||
| // TODO: implement splitting the entities into a class-structure for the frontend model | ||
| if (info.Length != 3) | ||
| { | ||
| // TODO: define how to proceed with this.. this is corrupted data! | ||
| continue; | ||
| } | ||
| var owner = info[0]; | ||
| var repoName = info[1]; | ||
| var prId = int.Parse(info[2]); | ||
|
|
||
| Repository repo; | ||
| var temp = user.RepositoryPrAddedTo | ||
| .FirstOrDefault(repo => repo.Owner.Equals(owner, StringComparison.OrdinalIgnoreCase) && | ||
| repo.Name.Equals(repoName, StringComparison.OrdinalIgnoreCase)); | ||
| if (temp != null) | ||
| { | ||
| repo = temp; | ||
| } | ||
| else | ||
| { | ||
| repo = new Repository(owner, repoName, entity.Url, new List<PullRequest>()); | ||
| add = true; | ||
| } | ||
| var prs = entities.Where(e => e.RowKey.StartsWith($"{owner}:{repoName}", StringComparison.OrdinalIgnoreCase)); | ||
| foreach (var pr in prs) | ||
| { | ||
| var id = int.Parse(pr.RowKey.Substring(pr.RowKey.LastIndexOf(':') + 1)); | ||
| if (!repo.Prs.Any(pr => pr.PrId == id)) |
There was a problem hiding this comment.
There is a lot of code happening in this method. I would like it decomposed into smaller units to keep it all more solid.
Layla-P
approved these changes
Oct 2, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implemented storing user info
Implemented getting user info (including nested classes structure for the viewmodel)
Not entirely happy with the code yet (there's room for optimization), but created PR so others can check, test and improve if necessary
Closes #16