Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(ember): add work item list #174

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

feat(ember): add work item list #174

wants to merge 7 commits into from

Conversation

Yelinz
Copy link
Member

@Yelinz Yelinz commented May 4, 2021

image
adds a new view to display the work items to the user

@Yelinz Yelinz requested review from anehx, czosel and velrest May 4, 2021 11:46
@Yelinz Yelinz force-pushed the work-items branch 2 times, most recently from 6623a1a to 7696fc6 Compare May 27, 2021 12:46
Copy link
Contributor

@velrest velrest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a short and sweet acceptance test?

ember/app/cases/detail/work-items/edit/index/controller.js Outdated Show resolved Hide resolved
ember/app/cases/detail/work-items/edit/index/template.hbs Outdated Show resolved Hide resolved
ember/app/components/work-item-list/item/component.js Outdated Show resolved Hide resolved
ember/app/services/notification.js Outdated Show resolved Hide resolved
ember/app/styles/components/work-item-list/index.scss Outdated Show resolved Hide resolved
ember/app/caluma-query/models/work-item.js Outdated Show resolved Hide resolved
@Yelinz Yelinz force-pushed the work-items branch 2 times, most recently from f7fe4a5 to 06fe10d Compare March 11, 2022 14:10
@Yelinz
Copy link
Member Author

Yelinz commented Mar 11, 2022

update and improvement of the work item list, removed unnecessary parts

relies on #210

@Yelinz Yelinz requested review from velrest and derrabauke and removed request for czosel March 11, 2022 14:12
@Yelinz Yelinz force-pushed the work-items branch 2 times, most recently from 510e2b8 to 88a37a5 Compare March 25, 2022 14:26
@Yelinz
Copy link
Member Author

Yelinz commented Mar 25, 2022

Refactored to ember-resources and added basic acceptance tests.
This is now ready for review

}

get case() {
return this.raw.case.parentWorkItem?.case || this.raw.case;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see where we're fetching the parentWorkItem

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

Comment on lines 22 to 23
# mirage
/mirage/mirage
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for Prettier users
pin eslint-plugin-prettier to 2.6.0, or
add the following to .eslintignore: /mirage/mirage

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you tried it without this? ember-caluma doesn't have this..

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

Comment on lines 6 to 9
const caseQuery = useCalumaQuery(this, allCases, () => ({
options: { pageSize: 1 },
filter: [{ id: case_id }],
}));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not how useCalumaQuery works. This needs to be a property on the controller.. Please check out the documentation in ember-caluma

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of fetching the same case/work item (duplicating this code snippet) in the child controllers, i opted to fetch them in the parent route model. What would be the best alternative to useCalumaQuery, just using apollo?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to apollo queries in controllers for both cases

Comment on lines 18 to 26
@dropTask()
*completeWorkItem() {
yield this.apollo.mutate({
mutation: completeWorkItem,
variables: { id: this.workItem.id },
});

this.actionButtonOnSuccess();
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the built-in <WorkItemButton /> or the action button question type

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to actionbutton

Comment on lines 7 to 10
const workItemsQuery = useCalumaQuery(this, allWorkItems, () => ({
options: { pageSize: 1 },
filter: [{ id: work_item_id }],
}));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, needs to be on the controller

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed

Comment on lines 9 to 10
<div uk-drop>
<div class="uk-card uk-card-body uk-card-small uk-card-default">
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you want to use the uk-dropdown here - not a card in a drop

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed

@@ -8,7 +8,8 @@
</LinkTo>

<UkTab as |tab|>
<tab.item @href="/">{{t "nav.documents"}}</tab.item>
<tab.item @href="/form-builder">{{t "nav.form-builder"}}</tab.item>
<tab.item @href="/">{{t "nav.cases"}}</tab.item>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<tab.item @href="/">{{t "nav.cases"}}</tab.item>
<tab.item @href="/" @linkToIndex={{true}}>{{t "nav.cases"}}</tab.item>

Copy link
Member Author

@Yelinz Yelinz Mar 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed

/>

<div class="uk-width-expand uk-flex uk-flex-middle uk-text-meta uk-margin">
<span>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why the span?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed

export default class WorkItemsController extends Controller {
queryParams = ["order", "status"];

// Filters
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that comment is very helpful, let's remove it

Copy link
Member Author

@Yelinz Yelinz Mar 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed


@action
updateFilter(type, value) {
set(this, type, value);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
set(this, type, value);
this[type] = value;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed

@Yelinz Yelinz force-pushed the work-items branch 2 times, most recently from 8fd0dce to 3f5f6a9 Compare April 6, 2022 08:00
@Yelinz Yelinz requested review from anehx and removed request for luytena and StephanH90 April 6, 2022 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants