Bug 156152 - [Sync View] Schedule a synchronize at night
Summary: [Sync View] Schedule a synchronize at night
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Team (show other bugs)
Version: 3.3   Edit
Hardware: PC Windows XP
: P5 enhancement (vote)
Target Milestone: 3.5 M2   Edit
Assignee: Platform Team Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: bugday, helpwanted
Depends on:
Blocks:
 
Reported: 2006-09-05 03:18 EDT by Benno Baumgartner CLA
Modified: 2009-06-02 05:31 EDT (History)
3 users (show)

See Also:


Attachments
Configure Sync Schedule dialog (14.95 KB, image/png)
2008-06-11 09:11 EDT, Tomasz Zarna CLA
no flags Details
Patch to add functionality as described in comment 1 (12.17 KB, patch)
2008-07-31 14:41 EDT, Trevor CLA
no flags Details | Diff
NumberFormatException error (5.50 KB, text/plain)
2008-08-04 10:41 EDT, Tomasz Zarna CLA
no flags Details
Updated Patch - null check on memento, and new default time (12.53 KB, patch)
2008-08-08 14:06 EDT, Trevor CLA
no flags Details | Diff
Patch adds 2 checkboxes for "Immediately" and "Run Once" and fixed prior issues (15.84 KB, patch)
2008-08-12 11:35 EDT, Trevor CLA
tomasz.zarna: iplog+
Details | Diff
Patch v04 (20.27 KB, patch)
2008-08-14 10:12 EDT, Tomasz Zarna CLA
no flags Details | Diff
mylyn/context/zip (49.18 KB, application/octet-stream)
2008-08-14 10:12 EDT, Tomasz Zarna CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Benno Baumgartner CLA 2006-09-05 03:18:36 EDT
I20060829-0800

I want to be able to schedule a synchronize every 24 hours, the synchronize should happen at night when I'm not at the machine. The only chance I have to acchive this with the current dialog is to spend a night in the office, which I rater not like to do:-) Please add a combo to say when the first synchronize should start, like:

Synchronize at: [00:00] o'clock
Repeat every: [24][hour(s)]

'Repeat every' could even have a checkbox to disable it. This way one could schedule a single synchronize.

Thanks
Comment 1 Michael Valenta CLA 2006-09-06 10:40:09 EDT
It is a good idea but it is not something we plan to do in the near future.
Comment 2 Trevor CLA 2008-06-06 13:55:43 EDT
Hello,

 I've put some work towards making this possible. 

 Mostly I've emulated the way the automatic update schedules its runs. However currently I use a preference which is probably not a good way to do it. 

 How best would something like this be exposed in the UI? I was thinking of putting it under Project Properties > CVS. Does this sound resonable? If so, how do I store the enablement state and time to update through that page?

 Thanks in advance
Comment 3 Tomasz Zarna CLA 2008-06-11 09:11:24 EDT
Created attachment 104474 [details]
Configure Sync Schedule dialog

I guess the dialog from the screen shot would be the best place. The class responsible for displaying it is ConfigureRefreshScheduleDialog in org.eclipse.team.ui. I guess the dialog stores the options that are already there, so I think you'll be able to do the same thing. In case you have any question, please ask.
Comment 4 Trevor CLA 2008-07-31 14:41:41 EDT
Created attachment 108891 [details]
Patch to add functionality as described in comment 1

I believe this patch is complete and does everything requested in the Description.
Comment 5 Tomasz Zarna CLA 2008-08-04 10:41:29 EDT
Created attachment 109070 [details]
NumberFormatException error

After applying the patch I saw the attached error in the Error log. This was the first time I opened the Sync view with the modified scheduler so I guess memento.getString(CTX_REFRESHSCHEDULE_START) returned null. It has to be checked before parsing. Looking forward to next version of the patch.
Comment 6 Trevor CLA 2008-08-08 14:06:39 EDT
Created attachment 109547 [details]
Updated Patch - null check on memento, and new default time

This is an update to the previous patch. I've added a check to see if the string from the memento is null, if it is, I don't set the start date. 

Also, the start date is now defaulting to 12:00:00 AM instead of the instantiation time of the feild. This seems better to me because it gives a nice (and consistant) time as opposed to 1:39:33 PM or something..
Comment 7 Tomasz Zarna CLA 2008-08-11 09:46:47 EDT
(In reply to comment #6)
> Created an attachment (id=109547)
> Updated Patch - null check on memento, and new default time
> 
> This is an update to the previous patch. I've added a check to see if the string
> from the memento is null, if it is, I don't set the start date.
> 
> Also, the start date is now defaulting to 12:00:00 AM instead of the
> instantiation time of the feild. This seems better to me because it gives a nice
> (and consistant) time as opposed to 1:39:33 PM or something..

The patch looks good, I do have some comments though:
1. I like the idea of setting the default start time to 12:00:00 AM, but what do you think about leaving the user an option to use current time? This could be done with a simple button called "Get Current Time" (or something similar). How does it sound?
2. There is a typo in the label for startTime widget. It should say "Synchronize at:".
3. The startTime widget should have border to match the other text fields on the dialogs.
Other than that, the patch works great. My only concern was the fact that the scheduler doesn't start the refresh job after restart, but it expected to work like that.
Comment 8 Tomasz Zarna CLA 2008-08-11 11:05:03 EDT
(In reply to comment #0)
> 'Repeat every' could even have a checkbox to disable it. This way one could
> schedule a single synchronize.

Trevor, would entering 0 for time interval give the same result? Or is the checkbox better in your opinion?
Comment 9 Trevor CLA 2008-08-12 11:32:41 EDT
(In reply to comment #7)
> (In reply to comment #6)
> > Created an attachment (id=109547) [details]
> > Updated Patch - null check on memento, and new default time
> > 
> > This is an update to the previous patch. I've added a check to see if the string
> > from the memento is null, if it is, I don't set the start date.
> > 
> > Also, the start date is now defaulting to 12:00:00 AM instead of the
> > instantiation time of the feild. This seems better to me because it gives a nice
> > (and consistant) time as opposed to 1:39:33 PM or something..
> The patch looks good, I do have some comments though:
> 1. I like the idea of setting the default start time to 12:00:00 AM, but what
> do you think about leaving the user an option to use current time? This could
> be done with a simple button called "Get Current Time" (or something similar).
> How does it sound?

Sounds like a good idea. The issue I see with this is if a user clicks "Get Current Time" and the time is set, then waits 2 seconds, that time is now in the past and the job is scheduled for Tomorrow at the choosen time. I believe a better option would be to have a "Immediately" checkbox which disables the DateTime widget and sets the date upon save. - This is what my next patch will do. 

 Also, the if one check immediately, then hits okay, the next time the dialog comes up, immediately is not checked and the startTime feild has the time previously calculated based on immediately.

> 2. There is a typo in the label for startTime widget. It should say
> "Synchronize at:".

Thank you for catching my typo. Fixed now.

> 3. The startTime widget should have border to match the other text fields on
> the dialogs.

Fixed.

> Other than that, the patch works great. My only concern was the fact that the
> scheduler doesn't start the refresh job after restart, but it expected to work
> like that.

(In reply to comment #8)
> (In reply to comment #0)
> > 'Repeat every' could even have a checkbox to disable it. This way one could
> > schedule a single synchronize.
> Trevor, would entering 0 for time interval give the same result? Or is the
> checkbox better in your opinion?

Entering 0 currently fails the validation check and it would only cause the job to run immediately after it finishes. I think the best way to accomplish this is with a checkbox.

Coming next is a patch with these enhancements incorperated.

Comment 10 Trevor CLA 2008-08-12 11:35:20 EDT
Created attachment 109795 [details]
Patch adds 2 checkboxes for "Immediately" and "Run Once" and fixed prior issues

patch to do as described in previous comment
Comment 11 Trevor CLA 2008-08-12 11:39:52 EDT
I missed one thing,

 after line 216 of ConfigureSynchronizeScheduleComposite.java there should be a call to updateEnablements();
Comment 12 Tomasz Zarna CLA 2008-08-14 10:12:28 EDT
Created attachment 109985 [details]
Patch v04

I like the "two checkboxes" approach. It works much better then my previous suggestions. Before releasing the patch I decided to make couple of changes:
1. Added updateEnablements(); as asked in comment 11
2. I decided to use a null value as an indicator that the sync job is about to start immediately.
3. Setting a refresh interval resets the runOnce flag to false;
4. In the setRunOnce method I replace the first startJob with stopJob, I guess that's what you meant ;)
5. The job has now a proper name in the progress view when ran in "Run Once" mode
6. "Run Once" string externalized. 
7. Added mnemonics to the dialog.
8. Added some javadocs and updated copyrights

What do you think? Does it still work as you would expect?
Comment 13 Tomasz Zarna CLA 2008-08-14 10:12:36 EDT
Created attachment 109986 [details]
mylyn/context/zip
Comment 14 Trevor CLA 2008-08-18 10:16:20 EDT
I've had a look at your newest patch and everything works as I expect and it looks very nice.

 I guess I hurried my last patch a bit. Thank You for cleaning it all up.
Comment 15 Tomasz Zarna CLA 2008-08-19 04:35:42 EDT
Thank you, this is still your contribution. Patch released to HEAD with a minor modification (start date is defaulting to 12:00:00 AM after ran immediately, as suggested in comment 6).
Comment 16 Pawel Pogorzelski CLA 2008-09-18 10:43:16 EDT
Verified on build I20080918-0100.
Comment 17 Szymon Brandys CLA 2008-09-18 10:47:20 EDT
VERIFIED by Pawel.