6
0
Fork 1

Fixed incorrect update of KNote "last modified" field caused by the lauch of KNotes. #40

Manuell gemergt
MicheleC hat 1 Commits von issue/38/knotes nach master vor 4 Jahren zusammengeführt
MicheleC hat vor 4 Jahren kommentiert
Besitzer

The journal last modified time was being incorrectly marked as updated when setting the color of the note. The color was set to the same values previously read from the configuration .ics file when loading a note at the launch of KNotes application.

This PR makes sure the journal is updated only is the colors are really changed, not at each call of setColor().

The journal last modified time was being incorrectly marked as updated when setting the color of the note. The color was set to the same values previously read from the configuration .ics file when loading a note at the launch of KNotes application.<br/> This PR makes sure the journal is updated only is the colors are really changed, not at each call of setColor().
MicheleC hat das Label PR/rfc vor 4 Jahren hinzugefügt
MicheleC hat diesen Issue vor 4 Jahren zum R14.1.0 release Meilenstein hinzugefügt
MicheleC hat vor 4 Jahren kommentiert
Ersteller
Besitzer

Ups... looks like Kontact is still messing up things. More work will be required to fix that. Anyhow if you can, please test the KNotes related part for the time being. I will work on the Kontact part tomorrow.

Ups... looks like Kontact is still messing up things. More work will be required to fix that. Anyhow if you can, please test the KNotes related part for the time being. I will work on the Kontact part tomorrow.
MicheleC hat den Titel von Fixed incorrect update of KNote "last modified" field caused by the lauch of KNotes. zu WIP: Fixed incorrect update of KNote "last modified" field caused by the lauch of KNotes. vor 4 Jahren geändert
deloptes hat vor 4 Jahren kommentiert
Mitarbeiter

Wouldn't it be better to rework knotes and kpart?

It might be more work, but at the end would be cleaner and I'm sure this problem will disappear.

I don't know if it is possible or how it would be possible, but it looks like it would require a backend to handle the notes and fromtend part - the application and another one - kpart_knotes.

It looks like to me that both are now somehow detached.

Wouldn't it be better to rework knotes and kpart? It might be more work, but at the end would be cleaner and I'm sure this problem will disappear. I don't know if it is possible or how it would be possible, but it looks like it would require a backend to handle the notes and fromtend part - the application and another one - kpart_knotes. It looks like to me that both are now somehow detached.
MicheleC hat vor 4 Jahren kommentiert
Ersteller
Besitzer

Hi Emanoil,

knotes_part is the KPart that can be embedded into other applications, like Kontact for example. KNotes is the original application, which I assume was developed before the Kpart thing.

Short term, I will work in fixing the kpart as well, I expect to be something similar to the first commit.

Long term solution would be to remove duplication of code and have KNotes to use the KPart as well, similar to what Kontact does. We can look into doing this after R14.1.0, to avoid further delay on the release of that. Also long term we need to work on a fix to get Knotes and Kontact sync together, to avoid changes in one app to be lost into the other (there is already issue #16 for that)

Hi Emanoil,<br/> knotes_part is the KPart that can be embedded into other applications, like Kontact for example. KNotes is the original application, which I assume was developed before the Kpart thing. Short term, I will work in fixing the kpart as well, I expect to be something similar to the first commit. Long term solution would be to remove duplication of code and have KNotes to use the KPart as well, similar to what Kontact does. We can look into doing this after R14.1.0, to avoid further delay on the release of that. Also long term we need to work on a fix to get Knotes and Kontact sync together, to avoid changes in one app to be lost into the other (there is already issue #16 for that)
MicheleC hat vor 4 Jahren kommentiert
Ersteller
Besitzer

PR updated with the fix for Kontact as well (just one extra line of code 😄).

Please test and let me know if it works for you too.

PR updated with the fix for Kontact as well (just one extra line of code :smile:).<br/> Please test and let me know if it works for you too.
MicheleC hat den Titel von WIP: Fixed incorrect update of KNote "last modified" field caused by the lauch of KNotes. zu Fixed incorrect update of KNote "last modified" field caused by the lauch of KNotes. vor 4 Jahren geändert
deloptes hat vor 4 Jahren kommentiert
Mitarbeiter

Hi Michele, regarding your previous post, it sounds like a very good plan.

Regarding this patch, I will try to test in the coming days, but at the end you again emit sigDataChanged whenever color was changed. I understand it is part of the note, but I am not sure it should be, however I would accept it. From first sight it looks promising.

regards

Hi Michele, regarding your previous post, it sounds like a very good plan. Regarding this patch, I will try to test in the coming days, but at the end you again emit sigDataChanged whenever color was changed. I understand it is part of the note, but I am not sure it should be, however I would accept it. From first sight it looks promising. regards
MicheleC hat vor 4 Jahren kommentiert
Ersteller
Besitzer

Hi Emanoil,

test here looks good so far, so I don't think emitting sigDataChanged is a problem. The problem was with the incorrect update of the journal entry, not the note itself.

Anyhow you may have more or different test cases from me, so you may run in different scenarios. If you testing is good as well, I will merge.

Hi Emanoil,<br/> test here looks good so far, so I don't think emitting sigDataChanged is a problem. The problem was with the incorrect update of the journal entry, not the note itself.<br/> Anyhow you may have more or different test cases from me, so you may run in different scenarios. If you testing is good as well, I will merge.
deloptes hat vor 4 Jahren kommentiert
Mitarbeiter

Hi Michele,

yes, tests were good.

Hi Michele, yes, tests were good.
SlavekB hat vor 4 Jahren kommentiert
Besitzer

This looks good and at first glance it seems to me that it can also be used for r14.0.x branch.

This looks good and at first glance it seems to me that it can also be used for r14.0.x branch.
MicheleC hat diesen Pull-Request vor 4 Jahren geschlossen
MicheleC löschte die Branch issue/38/knotes vor 4 Jahren
MicheleC hat das Label PR/rfc vor 4 Jahren entfernt
MicheleC hat den Meilenstein vor 4 Jahren von R14.1.0 release zu R14.0.8 release geändert
MicheleC hat vor 4 Jahren kommentiert
Ersteller
Besitzer

Thanks Emanoil and Slavek. Merged and backported to R14.0.8 too.

Thanks Emanoil and Slavek. Merged and backported to R14.0.8 too.
Dieser Pull Request wurde manuell als d1a4daaee6 gemergt.
Anmelden, um an der Diskussion teilzunehmen.
Keine Reviewer
Kein Meilenstein
Niemand zuständig
3 Beteiligte
Nachrichten
Fällig am

Kein Fälligkeitsdatum gesetzt.

Abhängigkeiten

Keine Abhängigkeiten gesetzt.

Referenz: TDE/tdepim#40
Laden…
Hier gibt es bis jetzt noch keinen Inhalt.