Fix regression in tag fetching for notifications
ClosedPublic

Authored by nowicki on Oct 29 2018, 12:50 PM.

Details

Summary

Commit 9ea06c01ba introduced some clean-up in tag fetching. Unfortunately with it came a regression which caused tags not to be fetched as part of notifications - no matter how the fetch scope is configured.

The problem lies in the fact that the FetchHelper class, which populates items based on the configured fetch scope still uses the Tags flag in the item fetch scope to determine if tags should be fetched at all and only if this flag is set the tag fetch scope is examined to determine what tag information to include.

On the notification side however support for handling this flag in the AggregatedItemFetchScope was removed, which causes the flag never to be set for the item fetch scope, which in turn means that no tags will ever be fetched for notifications regardless of tag fetch scope setting.

This commit brings back handling of the Tags flag in the item fetch scope thereby partially reverting commit 9ea06c01ba.

No autotests exist that cover this functionality. There is a Akonadi::Monitor test, but it does not test any tags and it looks like adding this would require supporting tags in knutresource, which is currently also missing.

Unfortunately it seems like the EWS resource is the only one that is really using tags and all the bugs eventually land on my table ;)

Test Plan

Test tag manipulation using the EWS resource and make sure that tags are in sync both locally and remotely.

Diff Detail

Repository
R165 Akonadi
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
nowicki created this revision.Oct 29 2018, 12:50 PM
Restricted Application added a project: KDE PIM. · View Herald TranscriptOct 29 2018, 12:50 PM
Restricted Application added a subscriber: kde-pim. · View Herald Transcript
nowicki requested review of this revision.Oct 29 2018, 12:50 PM
dvratil accepted this revision.Oct 31 2018, 9:07 AM
dvratil added a subscriber: dvratil.

Oh, sorry for that! And you are right, we should have more tests to cover this :( Many thanks for diving into Akonadi to find this.

This revision is now accepted and ready to land.Oct 31 2018, 9:07 AM
This revision was automatically updated to reflect the committed changes.