From kde-commits  Fri May 18 11:34:09 2007
From: David Faure <dfaure () klaralvdalens-datakonsult ! se>
Date: Fri, 18 May 2007 11:34:09 +0000
To: kde-commits
Subject: Re: branches/kdepim/enterprise/kdepim/kmail
Message-Id: <200705181334.18327.dfaure () klaralvdalens-datakonsult ! se>
X-MARC-Message: https://marc.info/?l=kde-commits&m=117948810902800

On Thursday 17 May 2007, Till Adam wrote:
> SVN commit 665722 by tilladam:
> 
> For read-only folders (those where we can't adjust the alarm relevance of a
> folder directly, offer to override teh alarm relevanve, that is, make it
> possible to block alarms for other people's folders. The gui changes from
> a combo box to a checkbox, in that case, and the "we have a local override"
> flag prevents the change from being overwritten by the next sync. If the
> alarms are re-enabled, that flag is reset, and the next sync restores the
> state from the server.
> 
>    if ( mIncidencesForComboBox )
> -      mIncidencesForComboBox->setEnabled( type == KMail::ContentsTypeCalendar ||
> -                                          type == KMail::ContentsTypeTask );
> +      mIncidencesForComboBox->setEnabled( enable );
> +  if ( mIncidencesForComboBox )
> +      mIncidencesForCheckBox->setEnabled( enable );

Copy/paste bug, fixed.

> -      KMFolderCachedImap::IncidencesFor incfor =
> -        static_cast<KMFolderCachedImap::IncidencesFor>( mIncidencesForComboBox->currentItem() );
> +    if ( folder->folderType() == KMFolderTypeCachedImap ) {
>        KMFolderCachedImap* dimap = static_cast<KMFolderCachedImap *>( mDlg->folder()->storage() );
> +      KMFolderCachedImap::IncidencesFor incfor = KMFolderCachedImap::IncForAdmins;
> +      if ( mIncidencesForComboBox ) {
> +        incfor = static_cast<KMFolderCachedImap::IncidencesFor>( mIncidencesForComboBox->currentItem() );
> +      }
> +      if ( mIncidencesForCheckBox ) {
> +        incfor = mIncidencesForCheckBox->isChecked() ? KMFolderCachedImap::IncForAdmins: KMFolderCachedImap::IncForNobody;
Can you explain the logic for this line?
This is about readonly folders; whether it's ForAdmins or ForNobody doesn't change anything for the current user, right?
So, shouldn't the checkbox toggle between "ForAllReaders" and one of the other two values?

>        if ( dimap->incidencesFor() != incfor ) {
>          dimap->setIncidencesFor( incfor );
> +        if ( mIncidencesForCheckBox && incfor != KMFolderCachedImap::IncForNobody )
> +          dimap->resetIncidencesForChanged();
This allows to clear the local override when the server has "for admins" and we select "for admins" locally.
But what about all the other cases?
Let's see..

* Server has ForNobody: should we then disable the checkbox? Currently it would be unchecked,
but if I check it then incfor will be ForAdmins (should be ForAllReaders, as mentionned above),
and the reset call should be made if I set back to Nobody (i.e. uncheck the checkbox), so the above if() has wrong logic.
I think we need to store a local copy of the server annotation?

* Server has ForAdmins: current user doesn't get alarms by default. So also the line
 mIncidencesForCheckBox->setChecked( dimap->incidencesFor() != KMFolderCachedImap::IncForNobody );
seems wrong...
AFAIU, in a readonly folder, the correct initialization logic would be
mIncidencesForCheckBox->setChecked( dimap->incidencesFor() == KMFolderCachedImap::IncForReaders );

* Server has ForReaders: current user gets alarms by default. And we need to let the checkbox
toggle between ForReaders and, well, either ForAdmins or ForNobody, doesn't matter since current
user gets no alarms in either case.

-- 
David Faure, faure@kde.org, dfaure@klaralvdalens-datakonsult.se
KDE/KOffice developer, Qt consultancy projects
Klarälvdalens Datakonsult AB, Platform-independent software solutions