dwyl / github-backup

:octocat: :back: 🆙 Backup your GitHub Issues so you can still work when (they/you are) offline.
https://github-backup.herokuapp.com
GNU General Public License v2.0
31 stars 3 forks source link

Create event to add additional repos after installation is complete #119

Closed RobStallion closed 5 years ago

RobStallion commented 6 years ago

If app has been installed on a repo and then a user would like to install it on another repo it currently errors.

This appears to be happening due to the missing (commented out) case of "installation_repositories".

RobStallion commented 6 years ago

Currently having an issue with testing.

I have written a test for the installation_repositories event that passes when run in conjunction with all the event tests, with the exception of the installation event test.

When it is run with the installation event test I get the following error...

     ** (Ecto.ConstraintError) constraint error when attempting to insert struct:

         * unique: issues_issue_id_index

Cannot see why this is happening at the moment as both tests work as expected individually.

SimonLab commented 6 years ago

The error is due to the constraint added on the id of the issue: https://github.com/dwyl/github-backup/blob/dcd0e6c9b4de744e4bd2b45e08cb79453c325900/priv/repo/migrations/20180319202333_add_constraints.exs#L5

We have already a test running for the event "installation": https://github.com/dwyl/github-backup/blob/dcd0e6c9b4de744e4bd2b45e08cb79453c325900/test/app_web/controllers/event_controller_test.exs#L7 with this test we trigger/send a payload to the server pretending to be a webhook event. This event will then run the new_installation function: https://github.com/dwyl/github-backup/blob/dcd0e6c9b4de744e4bd2b45e08cb79453c325900/lib/app_web/controllers/event_type_handlers.ex#L27 This function will get all the issues and comments for each new repositories where the app is installed and save the data into Postgres. However we don't actually send any API requests to Github to get the issues and comments but we mock them. We define first which Github API module to choose depending on the environment where the app is running: https://github.com/dwyl/github-backup/blob/dcd0e6c9b4de744e4bd2b45e08cb79453c325900/config/test.exs#L21 So for the test we use the InMemory module where the get_issues function is define like this: https://github.com/dwyl/github-backup/blob/dcd0e6c9b4de744e4bd2b45e08cb79453c325900/lib/app_web/controllers/github_api/in_memory.ex#L97-L99 The issue returned will then be saved on the database on "installation" Now when you try to run the installation_repositories event as you are using the same logic as the "installation" event the application will get the issue from the mock and try to insert it again in Postgres which will break because of the constraint.

I would suggest to find a way to return another issue for the "installation_repositories", maybe by updating the function in the InMemory module to return a specific mock issue depending on on of the parameter of the function? However there might be other nicer/clever solutions to the mock issue :thinking:

Or we can use insert_or_update to make sure we don't try to insert an issue which already exists in the database, I think this solution is better.

@RobStallion does all this makes sense?

RobStallion commented 6 years ago

@SimonLab Thanks a lot for explaining all of that and for the suggestion 👍. That makes much more sense now.