[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]