Add Reimbursement app #198
@ -8,8 +8,7 @@ contract Reimbursement is AragonApp {
|
||||
|
|
||||
bytes32 public constant VETO_REIMBURSEMENT_ROLE = keccak256("VETO_REIMBURSEMENT_ROLE");
|
||||
|
||||
struct ReimbursementData {
|
||||
address recordedBy;
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. 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).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
uint32 contributorId;
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. 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).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
uint32 recipientId;
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. 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).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
uint256 amount;
|
||||
address token;
|
||||
bytes32 hashDigest;
|
||||
@ -44,13 +43,12 @@ contract Reimbursement is AragonApp {
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. 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).
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. 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).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
}
|
||||
}
|
||||
|
||||
function get(uint32 reimbursementId) public view returns (uint32 id, address recordedBy, uint32 contributorId, uint256 amount, address token, bytes32 hashDigest, uint8 hashFunction, uint8 hashSize, uint256 confirmedAtBlock, bool exists, bool vetoed) {
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. 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).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
function get(uint32 reimbursementId) public view returns (uint32 id, uint32 recipientId, uint256 amount, address token, bytes32 hashDigest, uint8 hashFunction, uint8 hashSize, uint256 confirmedAtBlock, bool exists, bool vetoed) {
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. 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).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
id = reimbursementId;
|
||||
ReimbursementData storage r = reimbursements[id];
|
||||
return (
|
||||
id,
|
||||
r.recordedBy,
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. 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).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
r.contributorId,
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. 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).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
r.recipientId,
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. 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).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
r.amount,
|
||||
r.token,
|
||||
r.hashDigest,
|
||||
@ -62,14 +60,13 @@ contract Reimbursement is AragonApp {
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. 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).
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. 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).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
);
|
||||
}
|
||||
|
||||
function add(uint256 amount, address token, uint32 contributorId, bytes32 hashDigest, uint8 hashFunction, uint8 hashSize) public isInitialized auth(ADD_REIMBURSEMENT_ROLE) {
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. 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).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
function add(uint256 amount, address token, uint32 recipientId, bytes32 hashDigest, uint8 hashFunction, uint8 hashSize) public isInitialized auth(ADD_REIMBURSEMENT_ROLE) {
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. 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).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
uint32 reimbursementId = reimbursementsCount + 1;
|
||||
ReimbursementData storage r = reimbursements[reimbursementId];
|
||||
r.recordedBy = msg.sender;
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. 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).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
r.exists = true;
|
||||
r.amount = amount;
|
||||
r.token = token;
|
||||
r.contributorId = contributorId;
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. 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).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
r.recipientId = recipientId;
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. 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).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
r.hashDigest = hashDigest;
|
||||
r.hashFunction = hashFunction;
|
||||
r.hashSize = hashSize;
|
||||
|
||||
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
(Although I'm not sure if something like ```suggestion
event ReimbursementAdded(uint32 id, address indexed addedByAccount, uint256 amount);
```
(Although I'm not sure if something like `submittedBy` isn't nicer for a name there.)
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
Maybe we should just call this Maybe we should just call this `get`, analog to `add` below?
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. 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).
I think until we actually have the financial code done, we should not add this function or the I think until we actually have the financial code done, we should not add this function or the `claimed` property. 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).
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
yeah, thought about that, too. but we had yeah, thought about that, too. but we had `getContribution` in the contribution contract.
do you think that's the best pattern? :+1:
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
Yeah, I think it should be consistent, and Yeah, I think it should be consistent, and `contribution.add` is nicer for clients than `contribution.addContribution`. May as well imply the scope for all functions then. (We actually throw a deprecation warning for `addContribution` still. :))
|
||||
@ -23,7 +23,7 @@ class Reimbursement extends Record {
|
||||
async add (attrs, callOptions = {}) {
|
||||
const amount = parseInt(attrs.amount);
|
||||
const token = attrs.token;
|
||||
const contributorId = attrs.contributorId;
|
||||
const recipientId = attrs.recipientId;
|
||||
const expenses = attrs.expenses.map( e => new ExpenseSerializer(e) );
|
||||
let errorMessage;
|
||||
|
||||
@ -33,8 +33,8 @@ class Reimbursement extends Record {
|
||||
if (!token || token === '') {
|
||||
errorMessage = 'Invalid data: token must be a token address.';
|
||||
}
|
||||
if (!contributorId || contributorId === '') {
|
||||
errorMessage = 'Invalid data: contributorId is required.';
|
||||
if (!recipientId || recipientId === '') {
|
||||
errorMessage = 'Invalid data: recipientId is required.';
|
||||
}
|
||||
if (expenses.length === 0) {
|
||||
errorMessage = 'Invalid data: at least one expense item is required.';
|
||||
@ -50,7 +50,7 @@ class Reimbursement extends Record {
|
||||
const reimbursement = [
|
||||
amount,
|
||||
token,
|
||||
parseInt(contributorId),
|
||||
parseInt(recipientId),
|
||||
ipfsHashAttr.hashDigest,
|
||||
ipfsHashAttr.hashFunction,
|
||||
ipfsHashAttr.hashSize,
|
||||
|
||||
(Although I'm not sure if something like
submittedByisn't nicer for a name there.)(Although I'm not sure if something like
submittedByisn't nicer for a name there.)Maybe we should just call this
get, analog toaddbelow?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).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).yeah, thought about that, too. but we had
getContributionin the contribution contract.do you think that's the best pattern? 👍
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. :))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. :))