Add Reimbursement app #198
Loading…
x
Reference in New Issue
Block a user
No description provided.
Delete Branch "feature/expenses"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
refs #48, #195
To do
contributortorecipientNice! 👍
Left some comments/questions.
(Although I'm not sure if something like
submittedByisn't nicer for a name there.)Maybe we should just call this
get, analog toaddbelow?I think until we actually have the financial code done, we should not add this function or the
claimedproperty. Both because it will actually be out of sync with what we actually send around from the Gnosis safe, and also because we may want to adopt a different approach here as well (probably similar to the contributions, and maybe even combined with dividend payouts, so you only pay for a single tx when withdrawing everything at once).@ -39,2 +35,3 @@['Contribution', 'add', [{ contributorId: 2, contributorIpfsHash: 'QmcHzEeAM26HV2zHTf5HnZrCtCtGdEccL5kUtDakAB7ozB', date: '2019-04-11', amount: 1500, kind: 'dev', description: '[67P/kredits-web] Reviewed stuff', url: '' }, { gasLimit: 350000 }]],['Contribution', 'claim', [1, { gasLimit: 300000 }]],['Contribution', 'add', [{ contributorId: 1, contributorIpfsHash: 'QmWKCYGr2rSf6abUPaTYqf98urvoZxGrb7dbspFZA6oyVF', date: '2019-04-11', amount: 1500, kind: 'dev', description: '[67P/kredits-contracts] Add tests', url: '' }, { gasLimit: 350000 }]],['Contribution', 'add', [{ contributorId: 1, contributorIpfsHash: 'QmWKCYGr2rSf6abUPaTYqf98urvoZxGrb7dbspFZA6oyVF', date: '2019-04-11', amount: 1500, kind: 'dev', description: '[67P/kredits-contracts] Introduce contribution token', url: '' }, { gasLimit: 350000 }]],This seems inconsistent with the expense schema. I don't see how it creates the array of expenses, separate from the WBTC reimbursement. Would also be good to add two expenses for an example reimbursement from the start (ideally just the same example as in the schemas repo) for UI development.
Are these supposed to be committed?
arg, noo. of course not.
@ -39,2 +35,3 @@['Contribution', 'add', [{ contributorId: 2, contributorIpfsHash: 'QmcHzEeAM26HV2zHTf5HnZrCtCtGdEccL5kUtDakAB7ozB', date: '2019-04-11', amount: 1500, kind: 'dev', description: '[67P/kredits-web] Reviewed stuff', url: '' }, { gasLimit: 350000 }]],['Contribution', 'claim', [1, { gasLimit: 300000 }]],['Contribution', 'add', [{ contributorId: 1, contributorIpfsHash: 'QmWKCYGr2rSf6abUPaTYqf98urvoZxGrb7dbspFZA6oyVF', date: '2019-04-11', amount: 1500, kind: 'dev', description: '[67P/kredits-contracts] Add tests', url: '' }, { gasLimit: 350000 }]],['Contribution', 'add', [{ contributorId: 1, contributorIpfsHash: 'QmWKCYGr2rSf6abUPaTYqf98urvoZxGrb7dbspFZA6oyVF', date: '2019-04-11', amount: 1500, kind: 'dev', description: '[67P/kredits-contracts] Introduce contribution token', url: '' }, { gasLimit: 350000 }]],the array here is not an array of expenses but just how it is passed to the
addfunction. see above the seeds for contributions.The data here is not correct yet. and yep, we can add more just by c&p this line.
yeah, thought about that, too. but we had
getContributionin the contribution contract.do you think that's the best pattern? 👍
Yeah, I think it should be consistent, and
contribution.addis nicer for clients thancontribution.addContribution. May as well imply the scope for all functions then. (We actually throw a deprecation warning foraddContributionstill. :))@ -39,2 +35,3 @@['Contribution', 'add', [{ contributorId: 2, contributorIpfsHash: 'QmcHzEeAM26HV2zHTf5HnZrCtCtGdEccL5kUtDakAB7ozB', date: '2019-04-11', amount: 1500, kind: 'dev', description: '[67P/kredits-web] Reviewed stuff', url: '' }, { gasLimit: 350000 }]],['Contribution', 'claim', [1, { gasLimit: 300000 }]],['Contribution', 'add', [{ contributorId: 1, contributorIpfsHash: 'QmWKCYGr2rSf6abUPaTYqf98urvoZxGrb7dbspFZA6oyVF', date: '2019-04-11', amount: 1500, kind: 'dev', description: '[67P/kredits-contracts] Add tests', url: '' }, { gasLimit: 350000 }]],['Contribution', 'add', [{ contributorId: 1, contributorIpfsHash: 'QmWKCYGr2rSf6abUPaTYqf98urvoZxGrb7dbspFZA6oyVF', date: '2019-04-11', amount: 1500, kind: 'dev', description: '[67P/kredits-contracts] Introduce contribution token', url: '' }, { gasLimit: 350000 }]],Yes, that's not what I meant. I meant that a reimbursement contains an array of expenses, so it needs be an array for the
addfunction and the examples and seeds as well.@bumi Any news on this PR?
I think I'm running into the same issue as Travis: the deployment artifacts seem to be undefined (according to the error messages on Travis), resulting in the contract not being able to deployed during bootstrap on my machine:
FYI: I have updated the PR description with remaining tasks from the original issue as well as the Summit notes.
I am currently finishing up the Web UI for this.
67P/kredits-web/pull/195 is finished and ready for review/testing now. It will only need very small adjustments after finishing this PR.
do we need to save who added the reimbursement?
with the permissions it must be a contributor, but I had removed that we store who it is (mainly to avoid the lookup)
If we have the tx sender available in clients already, then we wouldn't need an additional attribute I guess. We just need that piece information in general. (Same for proposing kredits actually.)
I am not completely sure how to understand and explain the problem with deploying this aragon app.
When I try to deploy this app locally and install it into our DAO it seems to work but somehow it does not show up when I do
aragon dao apps ..and the JS also can not find the contract address.So far I failed to debug this issue.
Steps to do this:
aragon apm publish major --propagate-content=false --build=false --prepublish=false --skip-confirmationaragon dao install 0xbBc85e5dE36826e7fD52a871dB7e1735992e3AD4 kredits-reimbursement.open.aragonpm.etharagon dao apps 0xbBc85e5dE36826e7fD52a871dB7e1735992e3AD4I am not sure if that is related to #199
I don't fully understand. what do you want to store?
when adding an entry we have the
msg.senderwhich is the address. to lookup the contributor for that address we need to somehow talk to the contributor contract. This so far has always been a pain and I am not sure how to best connect the contracts to each other. (which is basically also the blocker still for deploying this)OK. But why is deploying the contract preventing the PR from being finished feature-wise and then merged? I think that should be done in a whole different PR, as that work is not about feature development for reimbursements.
I don't want to store anything that's not necessary. I simply mean that we need to be able to show who proposed a reimbursement later.
I just tried doing the last
kredits-webtask for this, but as this branch is now, I get exceptions from ethers when tryinggeta reimbursement, no matter from where. I wasn't able to fix it so far. Seeds run OK, but then it complains about insufficient data for assigning a boolean value tovetoedwhen loading a reimbursement in JS via the "native" contract function.Let's merge this, as it's working fine in dev, and we have to adapt everything for the production deployment and migration.