kredits/kredits-web
9
0
Fork 0
Code Issues 23 Pull Requests 1 Actions Releases 18 Activity

Finish Reimbursement UI (MVP) #209

Merged
raucao merged 7 commits from feature/veto_reimbursements into master 2023-01-09 03:34:37 +00:00
Conversation 3 Commits 7 Files Changed 9
+31 -1
3 changed files with 31 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 2082b51c5b - Show all commits
app/components/add-reimbursement/component.js
+17 -1
View File
@@ -6,6 +6,7 @@ import { action } from '@ember/object';
import { A } from '@ember/array';
import { scheduleOnce } from '@ember/runloop';
import isValidAmount from 'kredits-web/utils/is-valid-amount';
import readFileContent from 'kredits-web/utils/read-file-content';
import config from 'kredits-web/config/environment';
export default class AddReimbursementComponent extends Component {
@@ -85,7 +86,22 @@ export default class AddReimbursementComponent extends Component {
}
@action
updateContributor(event) {
async addExpensesFromFile (evt) {
const content = await readFileContent(evt.target.files[0]);
const expenses = JSON.parse(content);
if (expenses instanceof Array) {
for (const item of expenses) {
this.addExpenseItem(item);
}
} else {
console.warn("Expenses in file must be a list of items:");
console.debug(content);
}
}
galfert commented 2023-01-09 01:13:35 +00:00 (Migrated from github.com)
Review
Copy Link
Copy Source

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 commented 2023-01-09 03:33:50 +00:00 (Migrated from github.com)
Review
Copy Link
Copy Source

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.

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.
@action
updateContributor (event) {
this.recipientId = event.target.value;
}
app/components/add-reimbursement/template.hbs
+6
View File
@@ -87,3 +87,9 @@
<AddExpenseItem @addExpenseItem={{fn this.addExpenseItem}} />
{{/if}}
</form>
<form id="add-expenses-from-file">
<h3>Add expense items from file</h3>
<input type="file" multiple="false"
onchange={{fn this.addExpensesFromFile}}
accept="application/json" />
</form>
app/utils/read-file-content.js
+8
View File
@@ -0,0 +1,8 @@
export default function (file) {
const reader = new FileReader();
return new Promise((resolve, reject) => {
reader.onload = event => resolve(event.target.result)
reader.onerror = error => reject(error)
reader.readAsText(file)
})
}