The resource didn't implement collectionChanged at all.
Now it does; saving colors work. I realized this meant renaming
folders didn't work either. But that's for another commit because
there are dragons due to the tree structure being flattened.
Depends on D28937 to actually trigger the collection modify job.
Change color in korganizer, refresh roundcube, it shows the new color.
What I'm not sure about is what "Disable color" is supposed to do.
The code says "save an invalid color". OK... but what does it mean for the user?
Right now the calendar just gets some random color instead. What purpose does this serve?
Why not remove this action, which would remove a submenu i.e. improve usability?
| No Linters Available |
| No Unit Test Coverage |
| Buildable 25412 | |
| Build 25430: arc lint + arc unit |
| resources/dav/resource/davgroupwareresource.cpp | ||
|---|---|---|
| 613 | If the color is not valid, the job is basically a no-op. Could that potentially cause some issues? Maybe you should just add an else branch that calls changeCommitted() and returns immediately. | |
| resources/dav/resource/davgroupwareresource.cpp | ||
|---|---|---|
| 613 | That's *exactly* what I had initially. But as soon as this job does more than color -- like handling renamings -- then this approach doesn't work anymore. The only issue is a job that changes nothing, this doesn't give any errors. And since we can't know here which attributes of the collection have changed, this will happen many times. But see also my question about whether it actually makes sense to have an invalid color for a calendar, at all. From a user's point of view, I don't see it. | |
| resources/dav/resource/davgroupwareresource.cpp | ||
|---|---|---|
| 613 | Yeah, I also do not understand the point of "Disable Color" if it just ends up assigning a random one...IMO it can be removed. Regarding this code, guarding against invalid QColor still makes sense IMO to avoid breaking the DAV job in case there's something wrong stored in the configuration/Akonadi. | |