feat(query): create new work item dialog #2791

sahil143 · 2018-10-01T08:57:21Z

Note: If there are pending changes to the PR, prefix the PR title with "WIP" and add the label "DO NOT MERGE"

Mandatory

What does this PR do?
Add a way to create workitem from query page
What issue/task does this PR references?
https://openshift.io/openshiftio/Openshift_io/plan/detail/531
Are the tests Included?

Optional

Is the documentation Included?
Are the Release Notes included?

@mention(s) to expected reviewer(s) included

alien-ike · 2018-10-01T08:57:23Z

Ike Plugins (test-keeper)

Thank you @sahil143 for this contribution!

It appears that no tests have been added or updated in this PR.

Automated tests give us confidence in shipping reliable software. Please add some as part of this change.

If you are an admin or the reviewer of this PR and you are sure that no test is needed then you can use the command /ok-without-tests as a comment to make the status green.

Your plugin configuration is stored in the file.

divyanshiGupta · 2018-10-01T10:03:16Z

@sahil143 this PR has lint errors, please resolve them. And is the PR ready for review or is it work-in-progress?

divyanshiGupta · 2018-10-01T11:22:48Z

src/app/components_ngrx/planner-query/planner-query.component.html

+					[workItemTypes]="quickAddWorkItemTypes"
+					[selectedType]="quickAddWorkItemTypes[0]"
+					[selectedIteration]="selectedIterationSource | async">
+					</alm-work-item-quick-add>


Please indent alm-work-item-quick-add properly

divyanshiGupta · 2018-10-01T11:23:01Z

src/app/components_ngrx/planner-query/planner-query.component.less

+    }
+  }
+
+


Please remove extra lines

divyanshiGupta · 2018-10-01T11:23:08Z

src/app/components_ngrx/planner-query/planner-query.component.ts

@@ -202,6 +204,14 @@ export class PlannerQueryComponent implements OnInit, OnDestroy, AfterViewChecke
    });
  }

+  closeCreateWorkItemDialog(event) {


Please add the type for parameter event

divyanshiGupta · 2018-10-01T11:23:55Z

src/app/effects/work-item.effects.ts

@@ -14,6 +15,7 @@ import { WorkItemService as WIService } from './../services/work-item.service';
 import { AppState } from './../states/app.state';
 import * as util from './work-item-utils';

+


Remove this extra line

divyanshiGupta · 2018-10-01T11:24:07Z

src/app/effects/work-item.effects.ts

+                      type: NotificationType.SUCCESS
+                    } as Notification);
+                  } catch (e) {
+                    console.log('Work item is added.');


@sahil143 if the try block throws error that means the notification is not generated, so are you sure this is the correct way of handling it in catch ?

Yes, I think so. If the notification is not generated then the success will be logged on the console.

Do we really need try catch for notification service?

We should not use console logs anywhere in production.

Lastly, if we are using console logs then the user needs to open console to check if the work item was added. I don't think this is the correct user experience.

divyanshiGupta · 2018-10-01T11:28:24Z

@sahil143 functional tests are failing.

sahil143 · 2018-10-01T11:29:27Z

@sahil143 functional tests are failing.

Yes @divyanshiGupta I'm working on them

sahil143 · 2018-10-01T14:26:54Z

[test]

centos-ci · 2018-10-01T16:37:19Z

@sahil143 Your image is available in the registry. Run docker pull quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2791 && docker run -it -p 5000:8080 quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2791 and visit http://localhost:5000 to access it.

Raunak1203 · 2018-10-01T16:57:10Z

src/app/effects/work-item.effects.ts

-                // Add item success notification
+                let currentURL = document.location.pathname;
+                let detailURL = currentURL.indexOf('/plan/query') > -1 ? currentURL.split('/plan/query')[0] : currentURL;
+                const routeURL = detailURL + '/plan/detail/' + wItem.number;


@sahil143 do we need plan here in '/plan/detail/' ?

centos-ci · 2018-10-01T17:42:11Z

@sahil143 Your image is available in the registry. Run docker pull quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2791 && docker run -it -p 5000:8080 quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2791 and visit http://localhost:5000 to access it.

divyanshiGupta · 2018-10-01T18:23:55Z

src/app/effects/work-item.effects.ts

+                      type: NotificationType.SUCCESS
+                    } as Notification);
+                  } catch (e) {
+                    console.log('Work item is added.');


Do we really need try catch for notification service?

We should not use console logs anywhere in production.

Lastly, if we are using console logs then the user needs to open console to check if the work item was added. I don't think this is the correct user experience.

divyanshiGupta · 2018-10-01T18:25:08Z

src/app/components_ngrx/planner-query/planner-query.component.html

+				dropdownToggle>
+					Create WorkItem
+			</button>
+			 <div


There is still some weird indentation in this file.

centos-ci · 2018-10-01T20:00:31Z

@sahil143 Your image is available in the registry. Run docker pull quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2791 && docker run -it -p 5000:8080 quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2791 and visit http://localhost:5000 to access it.

sahil143 · 2018-10-22T07:24:22Z

[test]

sahil143 · 2018-10-22T07:25:03Z

[test]

Raunak1203 · 2018-10-23T05:38:47Z

[test]

centos-ci · 2018-10-23T06:24:22Z

@sahil143 Your image is available in the registry. Run docker pull quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2791 && docker run -it -p 5000:8080 quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2791 and visit http://localhost:5000 to access it.

centos-ci · 2018-10-23T06:39:29Z

@sahil143 Your image is available in the registry. Run docker pull quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2791 && docker run -it -p 5000:8080 quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2791 and visit http://localhost:5000 to access it.

centos-ci · 2018-10-23T06:39:54Z

@sahil143 Your image is available in the registry. Run docker pull quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2791 && docker run -it -p 5000:8080 quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2791 and visit http://localhost:5000 to access it.

divyanshiGupta · 2018-10-23T06:42:58Z

src/app/components_ngrx/work-item-quick-add/work-item-quick-add.component.html

+<!--Quick Add for Query Tab-->
+
+<div class="f8-quickadd-query__wrapper" *ngIf="wilistview === 'wi-query-view'">
+  <input class=""


Why is class attribute empty here? Or if we do not need this you can remove this.

divyanshiGupta · 2018-10-23T06:45:22Z

src/app/components_ngrx/work-item-quick-add/work-item-quick-add.component.ts

@@ -55,6 +56,7 @@ export class WorkItemQuickAddComponent implements OnInit, OnDestroy, AfterViewIn
  linkObject: object;
  addDisabled: Observable<boolean> =
    this.permissionQuery.isAllowedToAdd();
+  workItemTitle= new FormControl('');


add space between workItemTitle and =

divyanshiGupta · 2018-10-23T06:49:20Z

@sahil143 I have added a few review comments here and there apart from that the code looks good to me. And please make sure to remove console logs before merging this to master.

Raunak1203

@sahil143

After clicking create button creating a workitem popup should close.
Create and CreateAndOpen button is disabled after creating 1 workitem
Steps-

Create a workitem (It is displayed If it matches the query)
Try creating another work item , create button is disabled.

If a newly created work item is not matching the query, Can we display some notification saying that "Work Item is created but It doesn't match the Query"

sahil143 · 2018-10-23T09:00:05Z

@Raunak1203

If a newly created work item is not matching the query, Can we display some notification saying that "Work Item is created but It doesn't match the Query"

Newly created workitem is not shown at all in the list. we only show a notification.

Create and CreateAndOpen button is disabled after creating 1 workitem

This is fixed in commit 13efa60

After clicking create button creating a workitem dropdown should close.

No, It should not be because user might want to add another workitem right after a workitem is created

centos-ci · 2018-10-23T09:22:16Z

@sahil143 Your image is available in the registry. Run docker pull quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2791 && docker run -it -p 5000:8080 quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2791 and visit http://localhost:5000 to access it.

centos-ci · 2018-10-23T09:56:49Z

@sahil143 Your image is available in the registry. Run docker pull quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2791 && docker run -it -p 5000:8080 quay.io/openshiftio/fabric8-ui-fabric8-planner:SNAPSHOT-PR-2791 and visit http://localhost:5000 to access it.

sudsen · 2018-10-23T10:08:09Z

/ok-without-tests

sahil143 requested review from divyanshiGupta and sanbornsen and removed request for divyanshiGupta October 1, 2018 11:09

divyanshiGupta suggested changes Oct 1, 2018

View reviewed changes

Raunak1203 reviewed Oct 1, 2018

View reviewed changes

divyanshiGupta suggested changes Oct 1, 2018

View reviewed changes

sahil143 force-pushed the wi-create branch from 3bb12b2 to 18eb3bf Compare October 17, 2018 12:28

sahil143 added 11 commits October 23, 2018 11:23

fix(query): create new work item dialog

0dd10e5

fix(query-quick-add): change url for create and open

d7569a9

fix(workitemCreate): add close button to workitem create Dialog

19d5a24

chore(tslint): resolve linter error and remove dead code

460289c

fix(workitem-effect): Generate Success notification only for query Tab

cadeae0

chore(query): remove extra line and spaces

b958952

fix(query): temporarilt disable tests

63ef23f

fix(work-item.effect): change Add and open url for backlog tab

0527aa8

fix(query): fix indentation in query html

95c0027

fix(quickAdd): show the description of workitem type in dropdown

05a0614

fix(quickAdd): close popover on outside click

7f5ceb0

sahil143 added 2 commits October 23, 2018 11:23

fix(detailpage): preserve route when navigating to detail page

6b26e7d

fix(quickAdd): dont dispatch AddSuccess action on workitem creation

c8dc168

sahil143 force-pushed the wi-create branch from b2c271f to c8dc168 Compare October 23, 2018 05:53

divyanshiGupta suggested changes Oct 23, 2018

View reviewed changes

Raunak1203 suggested changes Oct 23, 2018

View reviewed changes

sahil143 added 3 commits October 23, 2018 13:59

fix(quickAdd): reset workitem popup after a workitem is created

13efa60

Merge Master into wi-create

51433ad

fix(query): add infinite scroll module removed during merge

e1b2366

fix(query): permission on query quick add

b63f323

Raunak1203 approved these changes Oct 23, 2018

View reviewed changes

sanbornsen approved these changes Oct 23, 2018

View reviewed changes

sudsen merged commit 2c33363 into fabric8-ui:master Oct 23, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(query): create new work item dialog #2791

feat(query): create new work item dialog #2791

sahil143 commented Oct 1, 2018

alien-ike commented Oct 1, 2018 •

edited

Loading

divyanshiGupta commented Oct 1, 2018

divyanshiGupta Oct 1, 2018

divyanshiGupta Oct 1, 2018

divyanshiGupta Oct 1, 2018

divyanshiGupta Oct 1, 2018

divyanshiGupta Oct 1, 2018

sahil143 Oct 1, 2018

divyanshiGupta Oct 1, 2018

divyanshiGupta commented Oct 1, 2018

sahil143 commented Oct 1, 2018

sahil143 commented Oct 1, 2018

centos-ci commented Oct 1, 2018

Raunak1203 Oct 1, 2018

centos-ci commented Oct 1, 2018

divyanshiGupta Oct 1, 2018

divyanshiGupta Oct 1, 2018

centos-ci commented Oct 1, 2018

sahil143 commented Oct 22, 2018

sahil143 commented Oct 22, 2018

Raunak1203 commented Oct 23, 2018

centos-ci commented Oct 23, 2018

centos-ci commented Oct 23, 2018

centos-ci commented Oct 23, 2018

divyanshiGupta Oct 23, 2018

divyanshiGupta Oct 23, 2018

divyanshiGupta commented Oct 23, 2018

Raunak1203 left a comment •

edited

Loading

sahil143 commented Oct 23, 2018

centos-ci commented Oct 23, 2018

centos-ci commented Oct 23, 2018

sudsen commented Oct 23, 2018

		@@ -14,6 +15,7 @@ import { WorkItemService as WIService } from './../services/work-item.service';
		import { AppState } from './../states/app.state';
		import * as util from './work-item-utils';

feat(query): create new work item dialog #2791

feat(query): create new work item dialog #2791

Conversation

sahil143 commented Oct 1, 2018

Mandatory

Optional

alien-ike commented Oct 1, 2018 • edited Loading

Ike Plugins (test-keeper)

divyanshiGupta commented Oct 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

divyanshiGupta commented Oct 1, 2018

sahil143 commented Oct 1, 2018

sahil143 commented Oct 1, 2018

centos-ci commented Oct 1, 2018

Choose a reason for hiding this comment

centos-ci commented Oct 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

centos-ci commented Oct 1, 2018

sahil143 commented Oct 22, 2018

sahil143 commented Oct 22, 2018

Raunak1203 commented Oct 23, 2018

centos-ci commented Oct 23, 2018

centos-ci commented Oct 23, 2018

centos-ci commented Oct 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

divyanshiGupta commented Oct 23, 2018

Raunak1203 left a comment • edited Loading

Choose a reason for hiding this comment

sahil143 commented Oct 23, 2018

centos-ci commented Oct 23, 2018

centos-ci commented Oct 23, 2018

sudsen commented Oct 23, 2018

alien-ike commented Oct 1, 2018 •

edited

Loading

Raunak1203 left a comment •

edited

Loading