[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]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic