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

Do not change created at during update #625

Merged
merged 2 commits into from Dec 26, 2016

Conversation

baijum
Copy link
Contributor

@baijum baijum commented Dec 23, 2016

fixes #623

Fields: Fields{},
}
res.Version = res.Version + 1
res.Type = wi.Type
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we do not need this Type assignment. (Can be skipped). Because we are not allowing type change yet.

Also, Fields are not reset. IMO we need to reset ?
While update API, we replace whole set of Fields with new input Fields but looking at this change we will be keeping old values in Fields.

Copy link
Contributor Author

@baijum baijum Dec 23, 2016

Choose a reason for hiding this comment

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

As of now, we are allowing the type change, but there is a discussion in UX to not allow. Until that is getting clarified let it be there.

Will reset Fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fields reset in 43af7d2

log.Printf("updated item to %v\n", newWi)
return convertWorkItemModelToApp(wiType, &newWi)
log.Printf("updated item to %v\n", res)
return convertWorkItemModelToApp(wiType, &res)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have a small test that will expose this issue and check if created_at is same as before after Update calls.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. Ref. 43af7d2

@codecov-io
Copy link

codecov-io commented Dec 23, 2016

Current coverage is 69.90% (diff: 100%)

Merging #625 into master will increase coverage by 0.23%

@@             master       #625   diff @@
==========================================
  Files            74         74          
  Lines          4264       4300    +36   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits           2971       3006    +35   
  Misses         1009       1009          
- Partials        284        285     +1   

Powered by Codecov. Last update 3b55102...271bb8e

Copy link
Contributor

@pranavgore09 pranavgore09 left a comment

Choose a reason for hiding this comment

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

Old code updates created_at after update API call (unwanted update)

       created_at       |          updated_at           
------------------------+-------------------------------
 0001-01-01 00:00:00+00 | 2016-12-26 06:22:19.714545+00
(1 row)

This PR prevents that
When item was created ->

postgres=# select created_at,updated_at from work_items where id = 5;
          created_at           |          updated_at           
-------------------------------+-------------------------------
 2016-12-26 06:23:21.265841+00 | 2016-12-26 06:23:21.265841+00
(1 row)

When item was updated ->

postgres=# select created_at,updated_at from work_items where id = 5;
          created_at           |          updated_at           
-------------------------------+-------------------------------
 2016-12-26 06:23:21.265841+00 | 2016-12-26 06:24:07.158964+00
(1 row)

@baijum baijum merged commit be9de7f into fabric8-services:master Dec 26, 2016
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.

Created date is set to wrong value during work item update
3 participants