Skip to content
This repository has been archived by the owner on May 7, 2021. It is now read-only.

fix(list view): Introduce parent details in the list view. #2291

Merged
merged 16 commits into from
Oct 30, 2017

Conversation

nimishamukherjee
Copy link
Collaborator

@nimishamukherjee nimishamukherjee commented Oct 19, 2017

Introduce parent information on the list view.
Visual design - https://redhat.invisionapp.com/share/YZDOM6S2R#/screens
Visual design with specs - https://redhat.invisionapp.com/share/YZDOM6S2R#/screens/255484840
Visual design for indentation - https://redhat.invisionapp.com/share/XQDLTAHCY#/screens/260232204

@fabric8cd
Copy link
Contributor

@nimishamukherjee fabric8/fabric8-ui:SNAPSHOT-PR-2291-9 is deployed and available for testing at https://fabric8-ui-fabric8-ui-pr-2291-fabric8-planner.badger.fabric8.io

@fabric8cd
Copy link
Contributor

@nimishamukherjee fabric8/fabric8-ui:SNAPSHOT-PR-2291-10 is deployed and available for testing at https://fabric8-ui-fabric8-ui-pr-2291-fabric8-planner.badger.fabric8.io

@jarifibrahim
Copy link
Member

@nimishamukherjee Take a look at this.
abc

@rgarg1
Copy link
Member

rgarg1 commented Oct 24, 2017

@nimishamukherjee This looks like a bug
super expanded list view

@rgarg1
Copy link
Member

rgarg1 commented Oct 24, 2017

@nimishamukherjee and this too
escaped chars in immediate parent title

@rgarg1 rgarg1 self-requested a review October 24, 2017 08:37
Copy link
Member

@rgarg1 rgarg1 left a comment

Choose a reason for hiding this comment

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

There are defects that need to be fixed.

@rgarg1
Copy link
Member

rgarg1 commented Oct 24, 2017

The tool-tip for the workitem's type of the parent is showing like this:
wit of immediate parent tooltip

@rgarg1
Copy link
Member

rgarg1 commented Oct 24, 2017

@nimishamukherjee Can we show the quick-preview of the immediate parent instead of the detail view, when the user clicks on the immediate parent ID? What does UX has to say about this?

@rgarg1
Copy link
Member

rgarg1 commented Oct 24, 2017

@nimishamukherjee We need a UX review too.

@rgarg1
Copy link
Member

rgarg1 commented Oct 24, 2017

One more issue. The Workitem type icon of immediate parent goes missing in the iteration context:

taskx-3 in list view - iteration

@jarifibrahim
Copy link
Member

@nimishamukherjee This looks broken to me. I see Task 2 3 times.
Also, the Task 2 has 2 parents. But only one shows up in the first entry.

abc1

@rgarg1
Copy link
Member

rgarg1 commented Oct 24, 2017

Any WI cannot have two parents. Its missing a check.

@jarifibrahim
Copy link
Member

@fabric8cd
Copy link
Contributor

@nimishamukherjee fabric8/fabric8-ui:SNAPSHOT-PR-2291-11 is deployed and available for testing at https://fabric8-ui-fabric8-ui-pr-2291-fabric8-planner.badger.fabric8.io

1. Display work item type tooltip for parents
2. Handle special characters for parent title
@fabric8cd
Copy link
Contributor

@nimishamukherjee fabric8/fabric8-ui:SNAPSHOT-PR-2291-14 is deployed and available for testing at https://fabric8-ui-fabric8-ui-pr-2291-fabric8-planner.badger.fabric8.io

@nimishamukherjee
Copy link
Collaborator Author

@nimishamukherjee Can we show the quick-preview of the immediate parent instead of the detail view, when the user clicks on the immediate parent ID? What does UX has to say about this?

@rgarg1 checked with UX on this. The parent needs to open in detailed view.

@nimishamukherjee
Copy link
Collaborator Author

nimishamukherjee commented Oct 25, 2017

@nimishamukherjee This looks broken to me. I see Task 2 3 times.
Also, the Task 2 has 2 parents. But only one shows up in the first entry.

@jarifibrahim This is a backend issue and @kwk is looking in to this. We could create an issue for the UI also, disable parent linking if the work item already has a parent.

kwk added a commit to fabric8-services/fabric8-wit that referenced this pull request Oct 25, 2017
@rgarg1
Copy link
Member

rgarg1 commented Oct 25, 2017

Issues I see:

  1. Child workitems when expanded, should be indented a little right to the parent. E.g. WI 25 in the attached image

  2. The assignee icon for myself is often displayed differently. See second image.

  3. The duplicate WI issue persists.

indent_issue

assignee icon

@rgarg1
Copy link
Member

rgarg1 commented Oct 25, 2017

@nimishamukherjee Reviewed!

aslakknutsen pushed a commit to openshiftio/saas-openshiftio that referenced this pull request Oct 25, 2017
to fabric8-services/fabric8-wit@c6fc618

As you can see there are quite a lot of changes (**83 files changed, 2914 insertions(+), 728 deletions(-)**) and the delta is quite huge. We might want to deploy more often to reduce this delta...

## Quiet CI (fabric8-services/fabric8-wit#1704)
    
Installs RPM more quietly, builds docker containers more quietly, and pulls docker images more quietly.


## `make dev` fails to start because of container connection issue on macOS (fabric8-services/fabric8-wit#1705)
    
remove the `network_mode` option in the containers
specify the host/port for the `auth` service to connect to its `db`, using the
internal Postgres port (5432) and using the container's hostname (the container
name itself)
same applies for the `core` service to connect to its `db`

Fixes fabric8-services/fabric8-wit#1705

## Random test failures because of work item creation error (fabric8-services/fabric8-wit#1676)
    
Using the `Last-Modified` response header when possible
Also, making sure that the list of spaces is always ordered by
`updated_at DESC` to get the most recents spaces first.

Move the `work item number sequence` in its own subpackage.

Refactored the test to use the TestFixture to initialise the
work items from the data set.

Using an `UPSERT` statement to avoid creating then updating
 the work item number sequence, which also avoids putting a `LOCK`
 on the row (using the `... FOR UPDATE` option in the `SELECT`
 statement)

Remove the `optionalKeywords` which contained the work item number
but failed when the 3rd work item was retrieved because its number
('3') would match the description. This worked in the past when the
number did not exist and the search was based on the ID (UUID).

Added some weight on the work item number (`*A`) when the URL matches
a known one and the number can be extracted.

Also, add a test to search by number using the `number:%d` query.

Adding a test to create work items concurrently

Refactor identities creation in test: using UUIDs of created identities instead of usernames
also, move a comment to `init()` where it belongs
also, comment on use of `:*A` in fulltext search

Also: move some log statements to `DEBUG` instead of `INFO`
to reduce the noise during test executions

Fixes fabric8-services/fabric8-wit#1676

## WIP sirupsen-case for fabric8-wit (fabric8-services/fabric8-wit#1672)
    
Goa 1.3 now seems to check length requirements in from the design.

* Update golden files

by running these commands:

$ go test -v github.com/fabric8-services/fabric8-wit/controller -run \
 TestSuiteWorkItemType -testify.m TestCreate -update

$ go test -v github.com/fabric8-services/fabric8-wit/controller -run \
 TestSuiteWorkItemType -testify.m TestShow -update

$ go test -v github.com/fabric8-services/fabric8-wit/controller -run \
 TestSuiteWorkItemType -testify.m TestValidate -update

* Switch postgres to registry.centos.org/postgresql/postgresql:9.6 in CI tests

* Forgot to pass F8_RESOURCE_REMOTE=1 when running remote tests WITH coverage

Fixes fabric8-services/fabric8-wit#1395

## Make link topology a type based on string (fabric8-services/fabric8-wit#1710)
    
* Make link topology a type based on string
* Rename CheckValidTopology to CheckValid and make it a member function of a Topology


## Simplify usage of test fixture customization functions (fabric8-services/fabric8-wit#1711)

## Added "not found" cases for search by number (fabric8-services/fabric8-wit#1712)
    
* Update search_repository_whitebox_test.go

Update TestSearch - By Number

Add "not found" case for when searching by space and not by space.

Added case with single and multiple results for when searching by number without a space.

## 'make dev' fails to start because of container connection issue on macOS (fabric8-services/fabric8-wit#1705) (fabric8-services/fabric8-wit#1713)
    
keep the previous changes in `docker-compose.macos.yml` for now
revert the changes as they break on Linux

Fixes fabric8-services/fabric8-wit#1705

## Improve the field type system and add tests (fabric8-services/fabric8-wit#1130)
    
We now support a "bool" type for fields in a work item type.

I've added a lot of tests for valid and invalid field types on both, repo- and controller-level.

The field type `KindWorkitemReference` has been removed.

## Do not import pkg 'notification' in pkg 'test' (fabric8-services/fabric8-wit#1716)
    
Move the 'test/notification.go' file into a subpackage
Rename the 'NotificationChannel' into 'FakeNotificationChannel'
to enforce its testing purpose.

Fixes fabric8-services/fabric8-wit#1716

## Work items disappear after unlinking (fabric8-services/fabric8-wit#1708)
    
* Work items disappear after unlinking (fabric8-services/fabric8-wit#1708)

Fix the SQL filter query by making sure only undeleted
links are taken into account when the `parentexists` filter
is activated.
Also, reduce the number of subselects  and add index
on table and improve query

Also, refactor the `search/search_repository_blackbox_test.go`
by using sub tests.

Also, make sure that tests in `TestSearchParentExists` include
the `userSpaceID` in the search expression.

Fixes fabric8-services/fabric8-wit#1708
    
## Creator filter (fabric8-services/fabric8-wit#1715)
    
Filter for work item creator

## Use just `state` instead of `workitemstate` (fabric8-services/fabric8-wit#1721)

## Only space collaborator and WI creators should be able to update/delete links (fabric8-services/fabric8-wit#1718)

## Enable Authorization by default (fabric8-services/fabric8-wit#1723)
        
Previously authorization had to be enabled explicitly in dev mode. With this change, WIT talks to AUTH which handles the authorization implementation details ( like talking to keycloak ).

## Copy links to relationships for `List space` endpoint (1/2) (fabric8-services/fabric8-wit#1687)
    
This ensure that the response will be compliant with the JSON-API specification.

Fixes fabric8-services/fabric8-wit#1687

##  Save description when new iteration created (fabric8-services/fabric8-wit#1726)
    
Fixes: https://openshift.io/openshiftio/openshiftio/plan/detail/1555

## Delete iteration API  (fabric8-services/fabric8-wit#1683)

Allow the user to delete the iteration (soft-delete) using API.

##  WIP: Make the parenting link type have a tree-topology (fabric8-services/fabric8-wit#1729)
    
Based on [this discussion](https://chat.openshift.io/developers/pl/gqwxx3mj13fejfdwqqc61mrwba) we decided to make the parenting link type have a tree topology.
    
See also fabric8-ui/fabric8-planner#2291 (comment)
@nimishamukherjee
Copy link
Collaborator Author

Child workitems when expanded, should be indented a little right to the parent. E.g. WI 25 in the attached image

@SMahil will fix this in a separate PR as we got the designs today from UX and she is on PTO tomorrow.

The assignee icon for myself is often displayed differently. See second image.

Create a separate issue for this as this is not related to this PR.

The duplicate WI issue persists.

Please refer to our Cabal discussion - https://docs.google.com/document/d/17LdWVYXoR4CXZUsRMq5qzpicQ49Yr0nR79lMYAumYIk/edit
The second topic addressed by me. Have a look at the comments.
Pasting the same here
Checked with Samir and he confirmed the following behaviour.
If we have a structure like
A
--B (B is a child of A)
C
--D (D is a child of C)
If we query by All work item types,
we should get
▸ A
B
▸ C
D

Clicking on A or C would take the user in to exploration mode and would display B again
▾ A
B
B
▾ C
D
D

@fabric8cd
Copy link
Contributor

@nimishamukherjee fabric8/fabric8-ui:SNAPSHOT-PR-2291-15 is deployed and available for testing at https://fabric8-ui-fabric8-ui-pr-2291-fabric8-planner.badger.fabric8.io

@rgarg1
Copy link
Member

rgarg1 commented Oct 26, 2017

@nimishamukherjee
Although I've my reservations against displaying a workitem twice, but since its approved by UX, lets mode ahead. BTW, has the behavior changed since I last looked at the preview?

Do we already have a defect to track the indentation issue or should I log one?

@nimishamukherjee
Copy link
Collaborator Author

Work items created to track issues on this PR

@nimishamukherjee
Copy link
Collaborator Author

nimishamukherjee commented Oct 27, 2017

@rgarg1 the behaviour has not changed.
Please make a note of the issues that are being tracked.

@fabric8cd
Copy link
Contributor

@nimishamukherjee fabric8/fabric8-ui:SNAPSHOT-PR-2291-16 is deployed and available for testing at https://fabric8-ui-fabric8-ui-pr-2291-fabric8-planner.badger.fabric8.io

@nimishamukherjee nimishamukherjee merged commit 928201d into fabric8-ui:master Oct 30, 2017
kwk added a commit to fabric8-services/fabric8-wit that referenced this pull request Oct 31, 2017
The following link types exist in the current production database but they are not known to the code and therefore we re-assign all links associated to those link types with their appropriate *known* link type and later remove these unknown link types.

aad2a4ad-d601-4104-9804-2c977ca2e0c1
355b647b-adc5-46b3-b297-cc54bc0554e6
7479a9b9-8607-46fa-9535-d448fa8768ab

There also exist two unknown link categories [see here](https://api.openshift.io/api/workitemlinkcategories):

04b89525-84ff-406c-8e18-d990936bdb74
6329fbb3-399b-4a78-ae3b-6a6faa8b1084

Those will be removed as well.

After this PR is merged, we should only have three link types to choose from in the UI and the duplicates should be gone as well. Without the duplicate entries of `parent of` and `child of` you then can no longer link a WI to two parents. Without this PR you still can link a child WI to two parent WIs when you choose from two different link types.

To double check, what link types exist, try this command:

```sh
curl --silent https://api.openshift.io/api/spaces/020f756e-b51a-4b43-b113-45cec16b9ce9/workitemlinktypes \
| jq ".data[] | .id, .attributes.name"
```

It should output the id and name of each link  type in the current production database:
```json
"aad2a4ad-d601-4104-9804-2c977ca2e0c1"
"Bug blocker"
"355b647b-adc5-46b3-b297-cc54bc0554e6"
"Related planner item"
"7479a9b9-8607-46fa-9535-d448fa8768ab"
"Parent child item"
"2cea3c79-3b79-423b-90f4-1e59174c8f43"
"Bug blocker"
"9b631885-83b1-4abb-a340-3a9ede8493fa"
"Related planner item"
"25c326a7-6d03-4f5a-b23b-86a9ee4171e9"
"Parent child item"
```

In 90c595e I have introduced fixed IDs for link types and link categories and that must have been the time when the old ones weren't deleted.

As part of this PR I've replaced many of the `panic` calls within the migration tests with normal test failures. I've also wrapped errors and added filenames of executed SQL files to the error output to increase *debuggability* (is that a word?). 

This relates to #1729  and fabric8-ui/fabric8-planner#2291 (comment)
@kwk
Copy link
Collaborator

kwk commented Oct 31, 2017

@nimishamukherjee and @rgarg1 FYI we should no longer have multiple parent links in production and we should no longer be able to produce one of them.

@nimishamukherjee nimishamukherjee deleted the display_parent branch November 21, 2017 16:15
xcoulon pushed a commit to fabric8-services/fabric8-common that referenced this pull request Jul 4, 2018
The following link types exist in the current production database but they are not known to the code and therefore we re-assign all links associated to those link types with their appropriate *known* link type and later remove these unknown link types.

aad2a4ad-d601-4104-9804-2c977ca2e0c1
355b647b-adc5-46b3-b297-cc54bc0554e6
7479a9b9-8607-46fa-9535-d448fa8768ab

There also exist two unknown link categories [see here](https://api.openshift.io/api/workitemlinkcategories):

04b89525-84ff-406c-8e18-d990936bdb74
6329fbb3-399b-4a78-ae3b-6a6faa8b1084

Those will be removed as well.

After this PR is merged, we should only have three link types to choose from in the UI and the duplicates should be gone as well. Without the duplicate entries of `parent of` and `child of` you then can no longer link a WI to two parents. Without this PR you still can link a child WI to two parent WIs when you choose from two different link types.

To double check, what link types exist, try this command:

```sh
curl --silent https://api.openshift.io/api/spaces/020f756e-b51a-4b43-b113-45cec16b9ce9/workitemlinktypes \
| jq ".data[] | .id, .attributes.name"
```

It should output the id and name of each link  type in the current production database:
```json
"aad2a4ad-d601-4104-9804-2c977ca2e0c1"
"Bug blocker"
"355b647b-adc5-46b3-b297-cc54bc0554e6"
"Related planner item"
"7479a9b9-8607-46fa-9535-d448fa8768ab"
"Parent child item"
"2cea3c79-3b79-423b-90f4-1e59174c8f43"
"Bug blocker"
"9b631885-83b1-4abb-a340-3a9ede8493fa"
"Related planner item"
"25c326a7-6d03-4f5a-b23b-86a9ee4171e9"
"Parent child item"
```

In 90c595ea I have introduced fixed IDs for link types and link categories and that must have been the time when the old ones weren't deleted.

As part of this PR I've replaced many of the `panic` calls within the migration tests with normal test failures. I've also wrapped errors and added filenames of executed SQL files to the error output to increase *debuggability* (is that a word?).

This relates to #1729  and fabric8-ui/fabric8-planner#2291 (comment)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants