Skip to content

Handle unknown repo case for group projects#46

Merged
Hamled merged 1 commit intomasterfrom
bugfix
Jul 15, 2017
Merged

Handle unknown repo case for group projects#46
Hamled merged 1 commit intomasterfrom
bugfix

Conversation

@Hamled
Copy link
Copy Markdown
Contributor

@Hamled Hamled commented Jul 10, 2017

When GitHub has a PR made from a repository/fork that has been deleted
it shows "unknown repository" as the source on the PR page and the
API does not include any repo information for the "head" section of the
PR data.

We now check whether that information is available from the API and
effectively ignore the PR if it does not exist. My understanding is that
this means the submission line for each associated student will continue
to be listed as unsubmitted, however I do not have a good test case for
this situation.

This fix only applies to the logic for group projects as the individual
project does not require the same repo information.

Note: The safety check code that I've put in the contributors_url
method could be cleaned up a bit if we switch to Ruby 2.3.0 or later,
which supports Hash#dig for exactly this purpose.

When GitHub has a PR made from a repository/fork that has been deleted
it shows "unknown repository" as the source on the [PR page][1] and the
API does not include any repo information for the "head" section of the
PR data.

We now check whether that information is available from the API and
effectively ignore the PR if it does not exist. My understanding is that
this means the submission line for each associated student will continue
to be listed as unsubmitted, however I do not have a good test case for
this situation.

This fix only applies to the logic for group projects as the individual
project does not require the same repo information.

Note: The safety check code that I've put in the `contributors_url`
method could be cleaned up a bit if we switch to Ruby 2.3.0 or later,
which supports `Hash#dig` for exactly this purpose.

[1]: Ada-C7/VideoStoreConsumer#8
@Hamled Hamled requested a review from kariabancroft July 10, 2017 22:04
@kariabancroft
Copy link
Copy Markdown
Collaborator

Looks good - thank you for addressing this. We could potentially still mark the person who submitted the PR as submitted (not using the group/collaborator functionality) - but I'm not sure if that will add any value

@Hamled
Copy link
Copy Markdown
Contributor Author

Hamled commented Jul 11, 2017

If the repo with the project code doesn't exist anymore (which I think is the only case that would trigger this bug), I don't think we should consider the PR as submitted.

@Hamled
Copy link
Copy Markdown
Contributor Author

Hamled commented Jul 11, 2017

Actually, it looks like all of the commits are still available somehow (they must be copied somewhere on GitHub's database when you make the PR), so it would still be possible for us to review it and write in feedback.

Should I update this code to mark the PR creator as having submitted, even if the original repo was removed?

Also, given that we technically still have access to each of the commits it actually is possible to get a list of all students involved in the PR. It'd involve a lot more API calls though unless we used the GraphQL API.

@Hamled
Copy link
Copy Markdown
Contributor Author

Hamled commented Jul 15, 2017

We're sticking with the current version of this fix for now.

@Hamled Hamled merged commit 8c5c698 into master Jul 15, 2017
@Hamled Hamled deleted the bugfix branch July 15, 2017 00:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants