Add temporary code for field rename #2333

jarifibrahim · 2018-10-29T07:41:31Z

~~This PR builds upon #2330. I will update this PR once #2330 is merged.~~

~~Blocked by #2345~~

See #2343

The PR -

Fixes tests in expression_compiler d0e6171
Skips full text search tests and tests that depend on database triggers 08a84da
The full text search depends on the TSV which are not yet fixed and the IfModifiedSince tests depend on the triggers defined on work_item table. We have not yet fixed a database and it's causing test failures. I have created an issue for this Re-introduce skipped tests #2332.
Adds replaceFieldName (incoming/request workitem payload) and addFieldName (outgoing/response payload) with tests 99154fc
The request and response payload for /workitem(s) endpoint has changed
Updates all the Golden Files 31d7ccf

Todo

Update /workitemtype endpoint response payload Update /workitemtype response for field rename #2335
Update /workitem/events endpoint response to return appropriate response (We can't send all the events twice) Add onField to the /workitem/event response #2337

codecov · 2018-10-29T07:51:38Z

Codecov Report

❗ No coverage uploaded for pull request base (osio-story-743-rename-fields@5300ce5). Click here to learn what that means.
The diff coverage is 100%.

@@                       Coverage Diff                       @@
##             osio-story-743-rename-fields    #2333   +/-   ##
===============================================================
  Coverage                                ?   70.12%           
===============================================================
  Files                                   ?      171           
  Lines                                   ?    16641           
  Branches                                ?        0           
===============================================================
  Hits                                    ?    11670           
  Misses                                  ?     3840           
  Partials                                ?     1131

Impacted Files	Coverage Δ
workitem/workitemtype.go	`73.62% <ø> (ø)`
controller/workitem.go	`81.72% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5300ce5...31d7ccf. Read the comment docs.

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

jarifibrahim · 2018-10-30T07:08:12Z

[test]

codecov-io · 2018-10-30T10:06:08Z

Codecov Report

❗ No coverage uploaded for pull request base (osio-story-743-rename-fields@b942caf). Click here to learn what that means.
The diff coverage is 100%.

@@                       Coverage Diff                       @@
##             osio-story-743-rename-fields    #2333   +/-   ##
===============================================================
  Coverage                                ?   70.18%           
===============================================================
  Files                                   ?      171           
  Lines                                   ?    16641           
  Branches                                ?        0           
===============================================================
  Hits                                    ?    11680           
  Misses                                  ?     3833           
  Partials                                ?     1128

Impacted Files	Coverage Δ
workitem/workitemtype.go	`73.62% <ø> (ø)`
controller/workitem.go	`81.72% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b942caf...fc9ed88. Read the comment docs.

Previously the [deployment failed](fabric8-services#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.

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.

kwk

@jarifibrahim overall it looks good I had some questions that were answered while reading through but maybe you can give a confirmation or a thumbs up when my conclusion was right.

controller/golden_files_test.go

controller/iteration_blackbox_test.go

controller/test-files/search/show/filter_assignee_null_show_after_update_work_item.golden.json

controller/test-files/work_item/list_children/ok.res.payload.golden.json

kwk · 2018-11-05T07:22:41Z

controller/test-files/work_item/update/area/area_wit.res.payload.golden.json

@@ -13,15 +13,15 @@
            "kind": "area"
          }
        },
-        "system.area": {
+        "system_area": {


The UI potentially makes use of these field information. @sudsen, do you parse the WIT description that the backend returns and do you search for fields inside of it?

See #2333 (comment)

@jarifibrahim In the comment you referred to @sudsen said that the UI ignores _ fields:

UI will simply ignore '_' values for now.

But here you're replacing a field and the UI will not be able to find the area, right?

Correct. This PR replaces system.area with system_area because /workitemtype response isn't fixed in this PR. I have changed in separately in #2335 (see the golden file https://github.com/fabric8-services/fabric8-wit/pull/2335/files#diff-d743767476fe0f409baa3070f7728acc)

controller/workitem_whitebox_test.go

workitem/expression_compiler_blackbox_test.go

…abric8-services#2345)

DhritiShikhar · 2018-11-13T09:09:07Z

controller/test-files/work_item_link/list/ok.res.paylpad.golden.json

@@ -87,11 +87,22 @@
        "system.description.rendered": "\u003cp\u003e\u003ccode\u003e(see function github.com/fabric8-services/fabric8-wit/controller_test.(*workItemLinkSuite).TestList in controller/work_item_link_blackbox_test.go)\u003c/code\u003e\u003c/p\u003e\n",
        "system.metastate": null,
        "system.number": 1,
-        "system.order": 1,


how is the system.order 1 here

1 looks wrong to me. I don't know why it was 1

@michaelkleinhenz

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.

…ric8-services/fabric8-wit into osio-story-743-rename-fields

…1-temp-code

baijum

LGTM

kwk

I guess, we can merge this to your branch.

* Rename system.* to system_* in a all files (#2330) * Update branch osio-story-743-rename-fields (#2350) * Add temporary code for field rename (#2333) * Add replaceFieldName and addFieldName with tests * Remove unnecessary log.Error from golden_files_test.go * Fix failing test in expression_compiler_blackbox_test.go * Skip FullTextSearch and iteration test until migrations are added * Add golden files with old and new field names * Add todo to skipped tests * Add foo.bar test for expression_compiler * Update /workitemtype response for field rename (#2335) * Change workitemtype response payload * Update golden files related to workitemtype response change * Add onField to the /workitem/event response (#2337) * Add event_name attribute to the event response * fix golden files related to workitem_event * Rename event_name to onField in event/show response * Update golden files * Add migrations for field name rename (#2340) * Add migration for workitem_type field renames * Add field rename migrations for work_items and work_item_revisions table * Enable skipped test for field rename and update TSV trigger (#2342) * Add migration for TSV vector and triggers for field renames * Change migration file names

jarifibrahim added 4 commits October 25, 2018 11:13

Rename system.* to system_* in few files

c98c52a

Rename system.* to system_* in some more files

96f5fb3

Rename rest of the files

852cc4f

fixup

6893e95

jarifibrahim self-assigned this Oct 29, 2018

alien-ike added the work-in-progress label Oct 29, 2018

jarifibrahim mentioned this pull request Oct 29, 2018

Golden file fixes for field rename #2331

Closed

jarifibrahim changed the base branch from master to osio-story-743-rename-fields October 29, 2018 08:00

jarifibrahim force-pushed the osio-story-743-part-1-temp-code branch 2 times, most recently from 0f6ea31 to 8fa69e3 Compare October 29, 2018 08:58

jarifibrahim mentioned this pull request Oct 29, 2018

Rename system.* to system_* in a some files #2330

Merged

jarifibrahim force-pushed the osio-story-743-part-1-temp-code branch 2 times, most recently from 1e42789 to 541a9b3 Compare October 29, 2018 09:17

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

bbd8f88

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

jarifibrahim force-pushed the osio-story-743-part-1-temp-code branch from 89388c4 to ceb3034 Compare October 29, 2018 12:10

jarifibrahim added 5 commits October 29, 2018 17:44

Add replaceFieldName and addFieldName with tests

99154fc

Remove unnecessary log.Error from golden_files_test.go

6dd63ae

Fix failing test in expression_compiler_blackbox_test.go

d0e6171

Skip FullTextSearch and iteration test until migrations are added

08a84da

Add golden files with old and new field names

31d7ccf

jarifibrahim force-pushed the osio-story-743-part-1-temp-code branch from ceb3034 to 31d7ccf Compare October 29, 2018 12:14

jarifibrahim requested review from kwk and DhritiShikhar October 30, 2018 07:34

jarifibrahim changed the title ~~WIP - Add temporary code for field rename~~ Add temporary code for field rename Oct 30, 2018

alien-ike removed the work-in-progress label Oct 30, 2018

kwk added 2 commits October 30, 2018 12:59

This was referenced Oct 31, 2018

Update /workitemtype response for field rename #2335

Merged

Support system.* fields and system_* fields in backend #2343

Closed

Re-introduce skipped tests #2332

Closed

Add support for system_* fields along with system.* fields #2341

Merged

kwk reviewed Nov 5, 2018

View reviewed changes

jarifibrahim requested a review from baijum November 8, 2018 12:52

Add todo to skipped tests

04b430f

jarifibrahim force-pushed the osio-story-743-part-1-temp-code branch from c105466 to 04b430f Compare November 9, 2018 10:00

jarifibrahim mentioned this pull request Nov 9, 2018

Rename SystemBoardColumns to BoardColumns in workitem relationships #2345

Merged

jarifibrahim added 4 commits November 9, 2018 15:49

fixup

9f1d82d

Add foo.bar test for expression_compiler

97d38cb

Rename SystemBoardColumns to BoardColumns in workitem relationships (f…

829eb51

…abric8-services#2345)

Merge branch 'master' into osio-story-743-part-1-temp-code

cff0a08

DhritiShikhar reviewed Nov 13, 2018

View reviewed changes

kwk and others added 4 commits November 13, 2018 10:16

Merge branch 'master' into osio-story-743-rename-fields

8809964

Merge branch 'osio-story-743-rename-fields' of https://github.com/fab…

0eb6ba8

…ric8-services/fabric8-wit into osio-story-743-rename-fields

Merge branch 'osio-story-743-rename-fields' into osio-story-743-part-…

75edbe3

…1-temp-code

baijum approved these changes Nov 20, 2018

View reviewed changes

fix TODO formatting

fc9ed88

kwk approved these changes Nov 26, 2018

View reviewed changes

jarifibrahim merged commit efb6a38 into fabric8-services:osio-story-743-rename-fields Nov 26, 2018

jarifibrahim mentioned this pull request Feb 21, 2019

Update core.yaml openshiftio/saas-openshiftio#1226

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add temporary code for field rename #2333

Add temporary code for field rename #2333

jarifibrahim commented Oct 29, 2018 •

edited

Loading

codecov bot commented Oct 29, 2018 •

edited

Loading

jarifibrahim commented Oct 30, 2018

codecov-io commented Oct 30, 2018 •

edited by codecov bot

Loading

kwk left a comment

kwk Nov 5, 2018

jarifibrahim Nov 5, 2018

kwk Nov 9, 2018

jarifibrahim Nov 9, 2018

DhritiShikhar Nov 13, 2018

jarifibrahim Nov 13, 2018

baijum left a comment

kwk left a comment

Add temporary code for field rename #2333

Add temporary code for field rename #2333

Conversation

jarifibrahim commented Oct 29, 2018 • edited Loading

codecov bot commented Oct 29, 2018 • edited Loading

Codecov Report

jarifibrahim commented Oct 30, 2018

codecov-io commented Oct 30, 2018 • edited by codecov bot Loading

Codecov Report

kwk left a comment

Choose a reason for hiding this comment

kwk Nov 5, 2018

Choose a reason for hiding this comment

jarifibrahim Nov 5, 2018

Choose a reason for hiding this comment

kwk Nov 9, 2018

Choose a reason for hiding this comment

jarifibrahim Nov 9, 2018

Choose a reason for hiding this comment

DhritiShikhar Nov 13, 2018

Choose a reason for hiding this comment

jarifibrahim Nov 13, 2018

Choose a reason for hiding this comment

baijum left a comment

Choose a reason for hiding this comment

kwk left a comment

Choose a reason for hiding this comment

jarifibrahim commented Oct 29, 2018 •

edited

Loading

codecov bot commented Oct 29, 2018 •

edited

Loading

codecov-io commented Oct 30, 2018 •

edited by codecov bot

Loading