gh4a icon indicating copy to clipboard operation
gh4a copied to clipboard

Review creation

Open Tunous opened this issue 6 years ago • 8 comments

Tunous avatar Sep 16 '17 16:09 Tunous

Right now this pull request is just a collection of ideas and I need help with some stuff.

I wanted to make review creation work like on GitHub web interface. That is: it should allow you to add comments one by one as pending and submit them at any time from any place (web or mobile).

The problem is that GitHub API doesn't provide a way to easily add new comments to pending review (Unless I missed that). It requires first to delete old pending review and then create new with all comments. You can see how I tested that here.

I feel like this approach is bad. If for some reason there was an error after old pending was deleted but before new was created the user could lose all their comments. Sadly I don't have ideas on how to work around this....

Tunous avatar Sep 16 '17 16:09 Tunous

The problem is that GitHub API doesn't provide a way to easily add new comments to pending review (Unless I missed that). It requires first to delete old pending review and then create new with all comments. You can see how I tested that here.

FWIW, I don't see a way to do this either. May be worth asking Github support about it?

I feel like this approach is bad. If for some reason there was an error after old pending was deleted but before new was created the user could lose all their comments. Sadly I don't have ideas on how to work around this....

Yes, I agree, this approach is not ideal (and maybe could be improved by submitting the new review before deleting the old one and storing the ID of the to-be-deleted review persistently, so that even if we crash, we could ask the user for deletion of the duplicate review). Having proper API support for this would be much preferable though, so I'd try asking Github support first.

maniac103 avatar Sep 26 '17 07:09 maniac103

and maybe could be improved by submitting the new review before deleting the old one and storing the ID of the to-be-deleted review persistently, so that even if we crash, we could ask the user for deletion of the duplicate review

I tried that and it doesn't work. It crashes saying that there can be only one pending review.

Tunous avatar Sep 26 '17 07:09 Tunous

I tried that and it doesn't work. It crashes saying that there can be only one pending review.

sigh Github support it is then?

maniac103 avatar Sep 26 '17 07:09 maniac103

Yeah I'll ask them later

Tunous avatar Sep 26 '17 07:09 Tunous

I've sent the message, waiting for reply.

Tunous avatar Sep 27 '17 10:09 Tunous

I've got a really great and detailed response from GitHub staff. In short, the answer is that it is possible to do what we are looking for by using GraphQL API.

I'll try to implement a test solution with all the information I've got. We would have to use this new API at some point anyway so it's good place to start.

@maniac103 any thoughts?

Tunous avatar Sep 28 '17 17:09 Tunous

I've pushed a commit with proof of concept implementation. With it, we can correctly create a new review or add new comments to existing review if it already exists.

Now the important part is the implementation of the GraphQL API library. I've used apollo-android for this as it seems to be the best (only?) option available currently. The actual usage is pretty messy and is missing proper error handling, lifecycle binding etc. but it should be easy to solve after we merge #698. Using rxJava here should remove a lot of boilerplate code and solve the mentioned issues.

Working with GraphQL API seems to be easy. The hardest part is finding stuff in documentation. After that, we can freely experiment in explorer, copy the created queries/mutations to code and use generated classes.

@maniac103 Check it out and let me know what you think.

Tunous avatar Sep 29 '17 09:09 Tunous