Finish Reimbursement UI (MVP) #209

2022-12-30T15:28:08Z

raucao commented

2022-12-30 15:28:08 +00:00

(Migrated from github.com)

Add reimbursement vetos
Remove empty tags when adding expense items (trailing comma e.g.)
Add IPFS inspect button to reimbursement list
Cache reimbursement additions/changes on incoming contract events

* Add reimbursement vetos * Remove empty tags when adding expense items (trailing comma e.g.) * Add IPFS inspect button to reimbursement list * Cache reimbursement additions/changes on incoming contract events

bumi (Migrated from github.com) reviewed 2022-12-30 15:28:08 +00:00

galfert (Migrated from github.com) approved these changes 2023-01-09 02:05:15 +00:00

galfert (Migrated from github.com) left a comment

Looking good. Just left one improvement suggestion.

app/components/add-reimbursement/component.js

						
				@@ -89,0 +98,4 @@

				      }

				    } else {

				      console.warn("Expenses in file must be a list of items:");

				      console.debug(content);

galfert (Migrated from github.com) commented

2023-01-09 01:13:35 +00:00

Async actions are an anti-pattern. While the asynchronous call (here readFileContent() is still in progress, one might navigate away and the component will be destroyed. After the asynchronous call finishes, any later statement trying to access this will then throw an exception.

One solution would be to use an ember-concurrency task instead.

Async actions are an anti-pattern. While the asynchronous call (here `readFileContent()` is still in progress, one might navigate away and the component will be destroyed. After the asynchronous call finishes, any later statement trying to access `this` will then throw an exception. One solution would be to use an `ember-concurrency` task instead.

raucao (Migrated from github.com) reviewed 2023-01-09 03:33:50 +00:00

app/components/add-reimbursement/component.js

						
				@@ -89,0 +98,4 @@

				      }

				    } else {

				      console.warn("Expenses in file must be a list of items:");

				      console.debug(content);

raucao (Migrated from github.com) commented

2023-01-09 03:33:50 +00:00

Thanks! I added a note with a link to your comment for now, since this is kind of an undocumented and hidden feature for someone who knows what they're doing as of now. Mostly just added it so I could easily migrate the existing data, even though it may be useful for other imports in the future. It's basically instant and you wouldn't navigate away while adding a reimbursement, so I think it's OK to leave it as is for now and spend time on more important things.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: kredits/kredits-web#209