0000998: review due wrong

(0002134)
ivan (administrator)
2009-10-25 11:11

Hi Paul, any chance you could email me a copy of your database?

(0002135)
paulh (administrator)
2009-10-25 11:18

Hi Ivan,

db in the post

(0002674)
paulh (administrator)
2010-03-13 17:01

Problem here seems to be because now(0 - lastreview is passed to format_workday_minutes which converts this into the friendly format though we're passing in a full 24hours per day so is out by at least 3x (we're also passing in weekends)

We need to run this through calculate_working_time() I think, but not convinced

(0002676)
ivan (administrator)
2010-03-13 21:31

Review due always worked on actual calendar days, not days worked, so I think calculate_working_time() would be wrong for this.

(0002684)
ivan (administrator)
2010-03-14 15:30

There were several issues with this, it looks like theres been some confusion in the past. Review Due is a special SLA target that uses actual days not working days. Things weren't helped by a very badly name function that calculated the time since the last review, I've renamed it.

Fixed in 3.x branch svn r6243

Fix ported to Git sit:master eb163ae

(0002687)
paulh (administrator)
2010-03-14 16:29

Hi,

Unfortunately r6243 doesn't resolve this :-(

I have a call in my test system which was opened "Saturday 4th Oct 08 @ 5:38pm" (so 1 year 5 months and 8 days) though the incident table still tells me "Review Due > 747 working days ago!" which is over two years ago which is obviously wrong as the incident hasn't been open for that long.

I could be clutching at straws here though I suspect somewhere along the line real days are being converted into working days (so 1 day is becoming 3 days)

Paul

(0002688)
ivan (administrator)
2010-03-14 16:37

OK I'll take another look. Shouldn't be any more working days calculations involved, but maybe I missed one. Just to double check do you get the same on the incidents popup page? Coz it's more or less the same code but copied rather than re-used.

(0002689)
paulh (administrator)
2010-03-14 16:45

HI,

I do get "Review Due Now!" on the incident pop up, theres no doubt the incident is due a review as I've not touched in since last June. reviewdue.png shows what I can see in SiT

(0002753)
ivan (administrator)
2010-03-20 16:55

I've made some more changes in 3.x branch svn r6292 and it does again look fixed, I've used some more data to test this out and seems ok to me now.

Would appreciate some testing to confirm. thanks.

(0002844)
ivan (administrator)
2010-03-23 21:29

git a7e584a

Issue History
Date Modified	Username	Field	Change
2009-10-24 19:34	paulh	New Issue
2009-10-25 11:11	ivan	Note Added: 0002134
2009-10-25 11:11	ivan	Status	new => feedback
2009-10-25 11:18	paulh	Note Added: 0002135
2009-12-24 12:53	paulh	Status	feedback => new
2010-02-20 19:52	ivan	Status	new => confirmed
2010-02-20 19:52	ivan	Target Version	=> 3.60 LTS
2010-03-13 17:01	paulh	Note Added: 0002674
2010-03-13 21:31	ivan	Note Added: 0002676
2010-03-14 15:04	ivan	Status	confirmed => assigned
2010-03-14 15:04	ivan	Assigned To	=> ivan
2010-03-14 15:30	ivan	Note Added: 0002684
2010-03-14 15:30	ivan	Status	assigned => resolved
2010-03-14 15:30	ivan	Fixed in Version	=> Current SVN
2010-03-14 15:30	ivan	Resolution	open => fixed
2010-03-14 16:29	paulh	Note Added: 0002687
2010-03-14 16:29	paulh	Status	resolved => feedback
2010-03-14 16:29	paulh	Resolution	fixed => reopened
2010-03-14 16:37	ivan	Status	feedback => assigned
2010-03-14 16:37	ivan	Note Added: 0002688
2010-03-14 16:43	paulh	File Added: reviewdue.png
2010-03-14 16:45	paulh	Note Added: 0002689
2010-03-20 16:55	ivan	Note Added: 0002753
2010-03-20 16:55	ivan	Status	assigned => resolved
2010-03-20 16:55	ivan	Resolution	reopened => fixed
2010-03-20 17:43	ivan	Tag Attached: needsporting
2010-03-23 21:29	ivan	Note Added: 0002844
2010-03-23 21:29	ivan	Tag Detached: needsporting
2010-03-27 11:16	ivan	Fixed in Version	Current SVN => 3.60 LTS
2010-03-27 12:30	ivan	Status	resolved => closed

Notes
(0002134) ivan (administrator) 2009-10-25 11:11	Hi Paul, any chance you could email me a copy of your database?

(0002135) paulh (administrator) 2009-10-25 11:18	Hi Ivan, db in the post

(0002674) paulh (administrator) 2010-03-13 17:01	Problem here seems to be because now(0 - lastreview is passed to format_workday_minutes which converts this into the friendly format though we're passing in a full 24hours per day so is out by at least 3x (we're also passing in weekends) We need to run this through calculate_working_time() I think, but not convinced

(0002676) ivan (administrator) 2010-03-13 21:31	Review due always worked on actual calendar days, not days worked, so I think calculate_working_time() would be wrong for this.

(0002684) ivan (administrator) 2010-03-14 15:30	There were several issues with this, it looks like theres been some confusion in the past. Review Due is a special SLA target that uses actual days not working days. Things weren't helped by a very badly name function that calculated the time since the last review, I've renamed it. Fixed in 3.x branch svn r6243 Fix ported to Git sit:master eb163ae

(0002687) paulh (administrator) 2010-03-14 16:29	Hi, Unfortunately r6243 doesn't resolve this :-( I have a call in my test system which was opened "Saturday 4th Oct 08 @ 5:38pm" (so 1 year 5 months and 8 days) though the incident table still tells me "Review Due > 747 working days ago!" which is over two years ago which is obviously wrong as the incident hasn't been open for that long. I could be clutching at straws here though I suspect somewhere along the line real days are being converted into working days (so 1 day is becoming 3 days) Paul

(0002688) ivan (administrator) 2010-03-14 16:37	OK I'll take another look. Shouldn't be any more working days calculations involved, but maybe I missed one. Just to double check do you get the same on the incidents popup page? Coz it's more or less the same code but copied rather than re-used.

(0002689) paulh (administrator) 2010-03-14 16:45	HI, I do get "Review Due Now!" on the incident pop up, theres no doubt the incident is due a review as I've not touched in since last June. reviewdue.png shows what I can see in SiT

(0002753) ivan (administrator) 2010-03-20 16:55	I've made some more changes in 3.x branch svn r6292 and it does again look fixed, I've used some more data to test this out and seems ok to me now. Would appreciate some testing to confirm. thanks.

(0002844) ivan (administrator) 2010-03-23 21:29	git a7e584a

Relationships