Handle null cases for Events.DIRTY

[ArnyminerZ]

Purpose

See #1659. Events.DIRTY can be null, but we are not correctly handling it with NOT dirty.

Short description

• Extended condition from NOT dirty TO (dirty is NULL OR dirty=0)
• Improved tests to verify this behavior.

Checklist

• The PR has a proper title, description and label.
• I have self-reviewed the PR.
• I have added documentation to complex functions and functions that can be used by other modules.
• I have added reasonable tests or consciously decided to not add tests.

[sunkup]

Neat. I think that should do it 👍 Since the queries are longer now I'd prefer to have them indented, but not sure if @rfc2822 agrees, so feel free to leave it like that.

[rfc2822]

Looks good!

I like that the expression is always in parentheses. We should also write all SQL keywords like IS NULL in upper-case. If there's mixed case, it could cause the impression that is NULL and IS NULL is something different.

Did you check whether Events.DIRTY is used somewhere else where this problem may occur, including in the synctools library?

[sunkup]

Thanks for indenting 👍

Did you check whether Events.DIRTY is used somewhere else where this problem may occur, including in the synctools library?

Took a quick look. We could mention it here:

• kdoc of LocalCollection.markNotDirty()
• kdoc of LocalCollection.removeNotDirtyMarked()

I don't think there are other places, but better check yourself too. Synctools looks okay.

[ArnyminerZ]

Improved kdoc and used capital letters

[sunkup]

Ah dammit. I just realise LocalCollection is generic and markNotDirty() and removeNotDirtyMarked() are also used by task lists and address books, so we should probably ubdate the whole kdoc in a generic way and not speak about events at all (maybe just in the override kdoc?).

[ArnyminerZ]

Yeah... the kdoc currently for LocalCollection's markNotDirty and removeNotDirtyMarked is specific for events. We should find a way to make it either generic, or ignore it completely and expect overrides to add it.

However, for example, LocalAddressBook.removeNotDirtyMarked already doesn't say anything. So I'd set it to something more generic...

[ArnyminerZ]

The issue is that there's RawContacts.DIRTY, Events.DIRTY, then Jtx has its own flag; tasks has _DIRTY.... So its hard to reference it in a "generic" manner. Maybe we should just write it down somehow in plain text

[sunkup]

Exactly. I would provide a generic plain text description in the interface and override the generic kdoc by adding kdoc to the implementations. There we can mention Events.DIRTY and RawContacts.DIRTY, etc respectively. Can you add that?

[rfc2822]

If it's only about the KDoc: or just "the dirty flag of the respective provider"?

[rfc2822]

Minor comments about tests

[Copilot]

Pull Request Overview

This PR fixes the handling of null values for the Events.DIRTY field by updating SQL WHERE clauses to properly check for both null and 0 values when identifying "not dirty" events.

• Updated SQL conditions from NOT dirty to (dirty IS NULL OR dirty=0) in both markNotDirty and removeNotDirtyMarked methods
• Added comprehensive test coverage for both dirty=0 and dirty=null scenarios
• Updated documentation to reflect the more accurate behavior description

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
LocalCollection.kt	Updated interface documentation to remove specific implementation details about dirty field handling
LocalCalendar.kt	Fixed SQL WHERE clauses to properly handle null dirty values in addition to 0 values
LocalCalendarTest.kt	Added comprehensive test cases to verify correct handling of both dirty=0 and dirty=null scenarios

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[rfc2822]

Looks good for me

[rfc2822]

(So ready to merge @ArnyminerZ :))