This repository has been archived by the owner on Mar 11, 2021. It is now read-only.
Ensuring API changes do not break Planner UI #2164
Comments
|
Thanks @jarifibrahim for your approval. |
|
@rgarg1 thank you for thinking about this. I just wanted to let you know that when we introduced space templates (which was the last major breaking change to me), planner did break not only because I did something wrong but also because the UI used the WIT API with false assumptions. To give an example, some endpoints where not taken from the relationships of a space but where hardcoded in the planner. I think this was the case with the work item types, correct @sudsen? The goal is to have a working system, so I'm not blaming anybody (including myself ;)) but we should also not have a look at the planner UI and see if a fix is required on that end as well, when one of your tests fail. |
|
@kwk Absolutely. We're also introducing additional checkpoints in prod-preview to reduce the chances of such problems escaping to production. Thanks for the approval. |
kwk
pushed a commit
that referenced
this issue
Aug 17, 2018
With this change the end to end tests are being executed on every PR for the fabric8-wit repo. Ref: #2164 Co-authored-by: Konrad Kleine 193408+kwk@users.noreply.github.com
|
Now that #2197 is merged, this issue could be closed. |
then close it? |
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.
2 tasks
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.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Goal: To run independently maintained Planner API tests on each WIT PR
Tests are here: https://github.com/fabric8io/fabric8-test/tree/master/EE_API_automation/pytest
Note: Currently, these tests run periodically against production:
https://ci.centos.org/job/devtools-test-end-to-end-openshift.io-planner-api-test/
Purpose: There have been instances in the past where a change in API has resulted in a failure of Planner UI in production. Any potential breaking changes in WIT is communicated to the UI team to ensure the production remains working, but this process is currently manual. I intend to automate this.
Ideally, to ensure no impact on UI, we should be running Planner UI tests on each WIT PR, but this would slow things down considerably so we don't want to run UI tests in WIT PRs.
Proposal: I propose to run these externally maintained API tests (aka API EE tests) on each PR instead. The tests would run as the last step on the PRs and excercise the Planner specific APIs just like a user would do. These tests also currently run on each Planner UI PR for the generation of test DB, and it also ensures that Planner UI's PR is compatible with the WIT source in prod-preview.
To catch issues early in WIT, we need to run these tests on each WIT PR. These tests are implemented in Python and take max 6 minutes to run.
Changes:
I plan to make changes to the following file to clone my test repo, install deps, start WIT, and run tests:
https://github.com/fabric8-services/fabric8-wit/blob/master/.make/test.mk
Approvals sought from:
The text was updated successfully, but these errors were encountered: