Skip to content
This repository has been archived by the owner on Jul 23, 2020. It is now read-only.

Agile template : Creating new defect has by default Priority P1, Severity- Sev1, and resolution-Done #3832

Closed
Raunak1203 opened this issue Jun 20, 2018 · 26 comments

Comments

@Raunak1203
Copy link

  • Create a new workItem of type Defect
  • Click on workitem to open quickPreview
  • By default Priority p1, Severity - Sev1, and resolution - Done selected

screenshot from 2018-06-20 11-26-59

@Raunak1203 Raunak1203 changed the title Agile template : Creating new defect has by default Priority P1, Sev1, and resolution-Done Agile template : Creating new defect has by default Priority P1, Severity- Sev1, and resolution-Done Jun 20, 2018
@Raunak1203 Raunak1203 changed the title Agile template : Creating new defect has by default Priority P1, Severity- Sev1, and resolution-Done Agile template : Creating new defect has by default shows Priority P1, Severity- Sev1, and resolution-Done Jun 20, 2018
@Raunak1203 Raunak1203 changed the title Agile template : Creating new defect has by default shows Priority P1, Severity- Sev1, and resolution-Done Agile template : Creating new defect has by default Priority P1, Severity- Sev1, and resolution-Done Jun 25, 2018
@Raunak1203
Copy link
Author

Raunak1203 commented Jul 2, 2018

this response is coming from backend

@jarifibrahim
Copy link
Collaborator

By default Priority p1, Severity - Sev1, and resolution - Done selected

We should have None/To Be Triaged field value as default

@rgarg1
Copy link
Collaborator

rgarg1 commented Jul 2, 2018

@Raunak1203

The default values should be set to None for fields:

  • Severity
  • Priority
  • Resolution

@michaelkleinhenz
Copy link
Collaborator

Yes, that is a known issue. But before we can fix this on the template, we need a UX consideration. So basically the issue is that for enum fields, the first value of a list is always the default and there is no "nil" value.

We could play around with the enum value list in the template, like reversing the values or adding a "n/a" "value", but that all appears to me like a hack. So we left this in for now. Let's have a discussion on how to properly treat "nil" values on enum fields..

@michaelkleinhenz
Copy link
Collaborator

@rgarg1 I'd say that is not always the case. So for example, a component field should never be empty. We need a general solution for this.

@Veethika
Copy link

Veethika commented Jul 2, 2018

IMO these fields should always hold a default value that's most commonly allocated in case of defects and are unalarming(like P1 and Sev1). The following could be considered to show as default:
Severity: Sev3 - Medium
Priority: P3 - Medium
Resolution: None
@catrobson @nimishamukherjee WDYT?
@michaelkleinhenz we would still need to discuss "how to properly treat "nil" values on enum fields" to solve the problem with 'Story-points'.

@rgarg1
Copy link
Collaborator

rgarg1 commented Jul 2, 2018

@Veethika
Having unalarming values as defaults is an acceptable solution. However, we still have to deal with None in the Resolution field and as you mentioned, story-points.

@michaelkleinhenz An empty field, screaming for attention sounds more acceptable to me than being populated with defaults, which was never updated by the author. But this is just my opinion.

@Veethika
Copy link

Veethika commented Jul 2, 2018

@rgarg leaving a field blank makes more sense when the content has to be unique for that field every single time, but for cases like these, pre-populating with some default value that fits the majority of the cases can make the job easier for the user.

@michaelkleinhenz
Copy link
Collaborator

That all feels to me like we need a proper "default value" handling in the backend. Currently, simply the first entry is treated as default. Which will not work with the above suggestions. To me it absolutely makes sense to have a default defined there explicitly. Like @Veethika has suggested. But that is actually a new story. I'll put it into the backlog.

@kwk
Copy link
Contributor

kwk commented Jul 3, 2018

That all feels to me like we need a proper "default value" handling in the backend. Currently, simply the first entry is treated as default. Which will not work with the above suggestions. To me it absolutely makes sense to have a default defined there explicitly. Like @Veethika has suggested. But that is actually a new story. I'll put it into the backlog.

@michaelkleinhenz We have the mechanics in place to have defaults. But what is blocking us on moving values up in an Enum to make them the default?

@rgarg1
Copy link
Collaborator

rgarg1 commented Jul 3, 2018

@michaelkleinhenz This is a SEV-2 and needs an early fix. As @kwk mentioned, this could be done.

@kwk
Copy link
Contributor

kwk commented Jul 5, 2018

I'm working on default values for all kind of fields and I'm almost finished.

@kwk
Copy link
Contributor

kwk commented Aug 20, 2018

I've made the changes to the template here in #2247:

https://github.com/fabric8-services/fabric8-wit/pull/2247/files#diff-ef9bca69129f8d8e357ad2ba0192455a.

The resolution is already set to None for a while already.

kwk added a commit to openshiftio/saas-openshiftio that referenced this issue Aug 21, 2018
----

**commit** fabric8-services/fabric8-wit@182f480
**Author:** Konrad Kleine <193408+kwk@users.noreply.github.com>
**Date:**   Thu Aug 16 21:53:58 2018 +0200

Remove unused migration test function (fabric8-services/fabric8-wit#2240)

The same thing is tested in migration 98 function.

----

**commit** fabric8-services/fabric8-wit@2b00c92
**Author:** Ruchir Garg <rgarg@redhat.com>
**Date:**   Fri Aug 17 15:37:15 2018 +0530

Add E2E API tests to PRs (fabric8-services/fabric8-wit#2197)

With this change the end to end tests are being executed on every PR for the fabric8-wit repo.

Ref: fabric8-services/fabric8-wit#2164

Co-authored-by: Konrad Kleine 193408+kwk@users.noreply.github.com

----

**commit** fabric8-services/fabric8-wit@95a5119
**Author:** Ibrahim Jarif <jarifibrahim@gmail.com>
**Date:**   Fri Aug 17 16:34:58 2018 +0530

Prevent double escaping of comments (fabric8-services/fabric8-wit#2236)

----

**commit** fabric8-services/fabric8-wit@b688f8f
**Author:** Ibrahim Jarif <jarifibrahim@gmail.com>
**Date:**   Mon Aug 20 12:23:21 2018 +0530

Workitem Type Change (fabric8-services/fabric8-wit#2202)

Implements story https://openshift.io/openshiftio/Openshift_io/plan/detail/43
This PR enables user to change workitem type.

A `PATCH` request on `/api/workitem/:uuid` with the following payload
```
"data": {
    "attributes": {
        "version": 1
    },
    "relationships": {
        "baseType": {
        "data": {
            "id": "00000000-0000-0000-0000-000000000001",
            "type": "workitemtypes"
        }
        }
    }
}
```
would change the type of workitem to `00000000-0000-0000-0000-000000000001`

**NOTE** - If the payload contains anything apart from the above fields the request will be rejected. We do not allow change of any attribute on a type change request. There's a followup story to address this: https://openshift.io/openshiftio/Openshift_io/plan/detail/433.

**Details** -
Consider the following two workitem types -
1. Type A has following fields
```
fooo - KindFloat
fooBar - KindEnum { KindString } Values { "open", "close", "done" }
assigned-to - KindList { KindUser }
bar - KindString
reporter - KindUser
```
2. Type B has following fields
```
fooo - KindFloat
bar - KindInteger
foobar - KindEnum { KindString } Values { "alpha", "beta", "gamma" }
```
If we have a workitem of `Type A` with following fields
```
WorkItem.Fields["fooo"] = 2.5
WorkItem.Fields["fooBar"] = "open"
WorkItem.Fields["bar"] = "hello"
WorkItem.Fields["reporter"] = "Jon Doe"
WorkItem.Fields["assigned-to"] = []string{"Jon Doe", "Lorem Ipsum"}
```

When the type of workitem is changed from `Type A` to `Type B`, the workitem will have the following fields
```
WorkItem.Fields["fooo"] = 2.5
WorkItem.Fields["fooBar"] = "alpha"
WorkItem.Fields["bar"] = null
WorkItem.Fields["system.description"] = "
======= Missing fields in workitem type: work item type Type A =======

Type1 Assigned To : Jon Doe, Lorem Ipsum
Type1 bar : hello
Type1 fooBar : open
Type1 reporter : Jon Doe

================================================"
```

1. Field `fooo` is present in `Type A` and `Type B` and that's why the new workitem has the same value set.
2. Field `bar` is present in `Type A` and `Type B` but the type is different (Type A has `string`, Type B has `int`). So, it is prepended to the description.
3. Field `fooBar` exists in both the types but the `Enum value` cannot be assigned to the new type. Hence it is prepended to the description.
4. Field `reporter` and `assigned-to` didn't exist in `Type B` and that's why they are added to the description.

**NOTE** -
1. ~~ All fields in the new workitem get a default value. ~~
2. All the relational fields (area, iteration, user) will be resolved. `UUIDs` will be resolved to get the actual value of the field and the actual value will be added to the description.
3. All the existing links will be preserved. We do not change/modify any existing links.

TODO
- [x] Disallow attribute change when workitem type is changed
- [x] Events API supports type change event (This will be done as a separate PR fabric8-services/fabric8-wit#2234)

## Additional notes

We support conversion of these field types as long as the individual field kinds match:

* simple to simple
* simple type to list
* simple type to enum
* list to list
* list to simple type
    * only when list contains just one element
* list to enum
    * only when list contains just one element
* enum to enum
* enum to simple type
* enum to list

That being said, we currently don't allow conversion from string to int fields for example.

Co-authored-by: Konrad Kleine 193408+kwk@users.noreply.github.com

----

**commit** fabric8-services/fabric8-wit@09d3745
**Author:** Michael Kleinhenz <kleinhenz@redhat.com>
**Date:**   Mon Aug 20 13:13:31 2018 +0200

feat(actions): Actions system infra

The actions system is a key component for process automation in WIT. It provides a way of executing user-configurable, dynamic process steps depending on user settings, schema settings and events in the WIT.

The current PR provides the basic actions infrastructure and an implementation of an example action rules for testing.

----

**commit** fabric8-services/fabric8-wit@87fdcec
**Author:** Ibrahim Jarif <jarifibrahim@gmail.com>
**Date:**   Mon Aug 20 17:30:06 2018 +0530

Add NotNull and NotEmpty constraint to Users.email column (fabric8-services/fabric8-wit#2248)

See fabric8-services/fabric8-wit#2237

----

**commit** fabric8-services/fabric8-wit@81c9706
**Author:** Michael Kleinhenz <kleinhenz@redhat.com>
**Date:**   Mon Aug 20 23:35:50 2018 +0200

Fixed migration test numbering and func order. (fabric8-services/fabric8-wit#2250)

This fixes the numbering of the migration tests where somehow, we started not using the migration order number from 100 to 102. This is somewhat confusing when adding new tests there.This change addresses this.

----

**commit** fabric8-services/fabric8-wit@cfe4545
**Author:** Ibrahim Jarif <jarifibrahim@gmail.com>
**Date:**   Tue Aug 21 14:23:07 2018 +0530

Add support for workitem type change event (#2234)

This PR depends on fabric8-services/fabric8-wit#2202 and fabric8-services/fabric8-wit#2239
Once the above mentioned PRs are merged I will rebase this pull request.


With this PR merged, the response for workitem type change event would look like

```yaml
{
  "data": [
    {
      "attributes": {
        "name": "workitemtype",
        "timestamp": "0001-01-01T00:00:00Z"
      },
      "id": "00000000-0000-0000-0000-000000000001",
      "relationships": {
        "modifier": {
              ....
        },
        "newValue": {
          "data": [
            {
              "id": "00000000-0000-0000-0000-000000000003",
              "type": "workitemtypes"
            }
          ]
        },
        "oldValue": {
          "data": [
            {
              "id": "00000000-0000-0000-0000-000000000004",
              "type": "workitemtypes"
            }
          ]
        },
        "workItemType": {
              ....
          },
      },
      "type": "events"
    }
  ]
}
```

----

**commit** fabric8-services/fabric8-wit@69ce1ac
**Author:** Konrad Kleine <193408+kwk@users.noreply.github.com>
**Date:**   Tue Aug 21 12:44:24 2018 +0200

Default value (fabric8-services/fabric8-wit#2247)

## Defaults for fields

This change introduces the ability for any field type to OPTIONALLY specify a custom default value. See also https://openshift.io/openshiftio/Openshift_io/plan/detail/35 and openshiftio/openshift.io#3832.

For lists and simple types, the only restriction is that the default value is of the same kind as the list or simple type (e.g. integer, float, string, user, label, ...). For enum fields the custom default value needs to be in the list of allowed values.

### Example usage of the feature

As an example, we've set the default for severity and priority fields to a medium value instead of the first on in the list, which would be a high severity/priority. (See also openshiftio/openshift.io#3832)

## Improvements to tests

### Space template import
When a space template is imported it creates or overrides the work item types. With this change I've introduced a cascading validation that checks

1. the work item types,
2. the fields inside each work item type
3. the field type of each field.

This ensures that people don't accidentally specify a float default (e.g. `33.45`) for an integer field.

Before this change, most of the input and output validation happened at runtime when a value was converted into another representation. Now, the validation happens at import time and also when a default value for a field is calculated.
@Veethika
Copy link

Veethika commented Oct 8, 2018

UX suggestion provided, removing UX labels.

@joshuawilson
Copy link
Member

@kwk I just looked at production and the resolution is set to Done.

@kwk
Copy link
Contributor

kwk commented Oct 29, 2018

@kwk I just looked at production and the resolution is set to Done.

In #3832 (comment) I wrote that the resolution has been set to None for a while already. As it turns out this was only true for the Impediment type but not for the Defect WIT.

I'll change that right away.

kwk added a commit to fabric8-services/fabric8-wit that referenced this issue Oct 29, 2018
Add `None` to a Defect's resolution in the Agile space template and make it the default resolution by putting it in first place.

See openshiftio/openshift.io#3832
@kwk
Copy link
Contributor

kwk commented Oct 29, 2018

@sanbornsen for the Impediment, we have a None resolution and now we have one for a Defect as well (should be there once fabric8-services/fabric8-wit#2334 is rolled out to prod-preview). I wonder if we should keep the None that was added by the UI. For an Impediment we currently see two Nones. Only one of them is selectable and the other is not.

bildschirmfoto von 2018-10-29 12-16-07

@kwk
Copy link
Contributor

kwk commented Oct 30, 2018

@sanbornsen We now have None as the default for all new Defects in prod-preview. The response from the work item creation shows that it is None for the resolution field. Do you know what problem there is within the UI when you see this screenshot?

bildschirmfoto von 2018-10-30 16-42-44

@kwk
Copy link
Contributor

kwk commented Oct 30, 2018

@sanbornsen I'm assigning the issue to you because I think that my part is done.

@jarifibrahim
Copy link
Collaborator

jarifibrahim commented Nov 1, 2018

@sanbornsen for the Impediment, we have a None resolution and now we have one for a Defect as well (should be there once fabric8-services/fabric8-wit#2334 is rolled out to prod-preview). I wonder if we should keep the None that was added by the UI. For an Impediment we currently see two Nones. Only one of them is selectable and the other is not.

bildschirmfoto von 2018-10-29 12-16-07

This issue was assigned to @sanbornsen but it was an easy one so I fixed it in fabric8-ui/fabric8-planner#2798

@joshuawilson
Copy link
Member

@kwk please don't assign issues to anyone but yourself. Unless done so in the context of sprint planning and only for your team.

@kwk
Copy link
Contributor

kwk commented Nov 2, 2018

@kwk please don't assign issues to anyone but yourself. Unless done so in the context of sprint planning and only for your team.

Why not? It is transparent for when and why it did it. I've talked about this before with @sanbornsen so I thought he knows best. If he doesn't want to work on it, he can assign it back or to someone else.

@joshuawilson
Copy link
Member

Because being assigned means you are actively working on it and no one else should. If he doesn't check this then it would just stay assigned to him and not get worked. In our planning cycle we assign all the work the individual should be able to do. If they have more bandwidth they can come look for unassigned bugs/issues and pick them up.

@kwk
Copy link
Contributor

kwk commented Nov 2, 2018

Because being assigned means you are actively working on it and no one else should. If he doesn't check this then it would just stay assigned to him and not get worked. In our planning cycle we assign all the work the individual should be able to do. If they have more bandwidth they can come look for unassigned bugs/issues and pick them up.

that is quite a static workflow, but okay.

kwk pushed a commit to openshiftio/saas-openshiftio that referenced this issue Nov 22, 2018
**Commit:** fabric8-services/fabric8-wit@bbd8f88
**Author:** Konrad Kleine (193408+kwk@users.noreply.github.com)
**Date:** 2018-10-29T12:14:17+01:00

Add 'None' as default resolution for defect (fabric8-services/fabric8-wit#2334)

Add `None` to a Defect's resolution in the Agile space template and make it the default resolution by putting it in first place.

See openshiftio/openshift.io#3832

----

**Commit:** fabric8-services/fabric8-wit@d070588
**Author:** Konrad Kleine (193408+kwk@users.noreply.github.com)
**Date:** 2018-10-30T12:59:57+01:00

Fixup for bbd8f88c789943293680644ae8a49b5342a55575 fabric8-services/fabric8-wit#2334 (fabric8-services/fabric8-wit#2336)

Previously the [deployment failed](fabric8-services/fabric8-wit#2334 (comment)):

```
failed to overwrite default of old field type with None (string):
failed to set default value of enum type to None (string):
value: None (string) is not part of allowed enum values:
[Done Duplicate Incomplete Description Can not Reproduce Deferred Won't Fix Out of Date Verified]
file: spacetemplate/importer/repository.go
line: 91
```

We've updated the resolution enum to
have a new value and that is also the new default. That new value didn't
exist in the old enum type but we tried to make it the new default for
the old type anyways. That didn't work because the `FieldType.SetDefault()`
implementation for enums checks if the given value is part of the
allowed enum values.

The overall intention is to check if too enums are the same but ignore
the default value. That is why we temporarily make both defaults the
same before we call `FieldType.Equal()`.

With this change we simply reverse the assignment of the new default to
the old type. Instead we temporarily assign the old default to the new
type. The result is that a call to `FieldType.Equal()` will return true.

----

**Commit:** fabric8-services/fabric8-wit@9dc1c08
**Author:** Konrad Kleine (193408+kwk@users.noreply.github.com)
**Date:** 2018-10-30T15:53:19+01:00

Added .github/CODEOWNERS file (fabric8-services/fabric8-wit#2339)

We want to use this file in Github to automatically assign reviewers to PRs if they are declared as "owners".

Read more about this file here:

* https://blog.github.com/2017-07-06-introducing-code-owners/
* https://help.github.com/articles/about-codeowners/

So far I've added only "my" parts and the ones I feel deeply responsible for.

We can add more code owners later.

----

**Commit:** fabric8-services/fabric8-wit@829eb51
**Author:** Ibrahim Jarif (jarifibrahim@gmail.com)
**Date:** 2018-11-12T19:22:40+05:30

Rename SystemBoardColumns to BoardColumns in workitem relationships (fabric8-services/fabric8-wit#2345)

----

**Commit:** fabric8-services/fabric8-wit@5205666
**Author:** Konrad Kleine (193408+kwk@users.noreply.github.com)
**Date:** 2018-11-13T10:16:37+01:00

capture range variable for parallel testing (fabric8-services/fabric8-wit#2346)

Before only the last range variable was being used and some tests just passed when the shouldn't have.

See also https://gist.github.com/posener/92a55c4cd441fc5e5e85f27bca008721.

And https://golang.org/pkg/testing/#hdr-Subtests_and_Sub_benchmarks for capturing range variables.

Thanks to @michaelkleinhenz for finding this and @jarifibrahim for finding the solution which is well known but in this case it is just too easy to miss.

----

**Commit:** fabric8-services/fabric8-wit@c81a4d2
**Author:** Preeti Chandrashekar (preetipagad@gmail.com)
**Date:** 2018-11-17T16:34:10+05:30

Adds infotips for Work Items WIG (fabric8-services/fabric8-wit#2266)

----
@jarifibrahim
Copy link
Collaborator

This issue has been fixed on prod.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests