[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: branches/kdepim/enterprise/kdepim/kmail
From:       David Faure <dfaure () klaralvdalens-datakonsult ! se>
Date:       2007-05-18 11:34:09
Message-ID: 200705181334.18327.dfaure () klaralvdalens-datakonsult ! se
[Download RAW message or body]

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


[prev in list] [next in list] [prev in thread] [next in thread]