[FIX #185] Readd bounty label

[vitvly]

FIX #185

Description

This fix does the following:

1. Listen to unlabeled events from GitHub API.
2. If such event is received, find the associated issue in the DB and remove it only if funds are zero (currently it checks the field value_usd field). This removes the bounty from SOB.
3. If it is successfully deleted from the DB, post to GitHub API a request to delete a comment. This will remove a bounty comment on GitHub.
4. No actions on part of the contract itself are done.

Questions

1. Is value_usd the correct field to check against when removing a bounty?
2. Should anything be done with regards to the bounty contract when the parent comment is deleted? If so, what messages should be sent?

Status: Finished.

[vitvly]

Changed the comment deletion logic in the DB so that it moves a deleted record to issues_archived table instead of dropping it altogether.

[pablodip]

Not sure about the current strategy, since having a similar table creates dependency on the original one, so when we want to modify/split the original table we would also have to do it with the archived table, and in the archived data.

The current goal is to just do not lose information, so I agree with the archive concept, just how about implementing it with a more general approach, for example an archive table with four columns, id, type, archived_at and data, and data could just be the edn of the data that we want to archive? That way it would be more open to archive anything, and we would not have the dependency with the info that we want to archive, so for example the archive would work with any version of the original data. The archived data would not be queriable from SQL, but we would need to take it from there and query it in Clojure, but it is an archive in the end.

Anyway if we seriously aim to not lose information and query better all data over time, that is, to have a time model for the data, we should probably consider using Datomic in the future.

[vitvly]

Changed the code so that deleted issues go to archive table. When deploying new contracts, this table is checked to fetch free contracts that haven't been assigned to any issue. So it fulfills a double purpose: that of an archive and a contract pool.

[vitvly]

Agree.

[martinklepsch]

This seems good but I'm also a little bit worried about introducing a separate table. Wouldn't something like revoked_at as suggested by @pablodip be easier to work with?

Thinking about https://github.com/status-im/open-bounty/issues/284 we'd need to

• read the JSON from the archive and parse that
• handle old versions as the table schema might change over time but the JSON blob stays the same

Are there any specific benefits with the separate table approach?

[vitvly]

Good point! Initially I've opted for archive simply because it was a less intrusive code change and did not imply changing multiple queries and associated middleware, hence smaller probability of regressions. Smart vocabulary aside, I was plain lazy:)

Still, can think of 2 things in favor of archive approach:

1. We'd like to have a DB model that works in an append-only fashion. No data can be deleted without a trace, so that we can recover what happened to each issue and when (audit trail capability). Currently the DB is append-only with regards to issues and pull requests - data is always appended. The archive can record multiple deletions of a single issue's bounty, while revoked_at will record the most recent one.
2. Related to a concern that archive serves a double purpose: @pablodip suggested modeling contracts in a ethereum_contract table, and changing issues to include a relation to ethereum_contract (https://github.com/status-im/open-bounty/issues/267). This would relieve archive of non-archival related duties.

Actually, the whole audit-trail related thing is important and requires more thought.

[pablodip]

To me the reason to not face re-modelling here isn't just about laziness, but about strategy. Introducing new tables or fields requires extra work in some existing things, and so might produce bugs as well. And we already intend to re-model with a different structure. So facing re-modelling here and in the new structure isn't probably needed. Besides, the feature that depends on the archive is an optional feature (re-use a contract) and easily refactored if needed.

[martinklepsch]

Actually, the whole audit-trail related thing is important and requires more thought.

Agree. Event Sourcing may be an interesting approach for this as well and is fundamentally similar to how Datomic works. Given the complexities around this issue my feeling is that we should probably "ignore it" for now and later on allocate some time to think about how to implement it properly.

[pablodip]

Agree. Event Sourcing may be an interesting approach for this as well and is fundamentally similar to how Datomic works.

I personally don't think they're similar, since Event Sourcing is just about modelling and persisting round domain events, while Datomic is a general-purpose database that adds the time dimension.

[churik]

1. GH comment is not posted after deploying contract

Steps: Requirements: GH account is whitelisted, signed app, test application is added to repo;

• navigate to repo
• create an issue
• add bounty label
• open https://testing.openbounty.status.im/app
• wait until new bounty will be displayed in 'Open bounties' list
• click on bounty

Actual result:

GH comment with contract details

Expected result:

no GH comment

Video: http://take.ms/SMVwj OS: Mac OS High Sierra Browser: Chrome 64