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

Fusionné manuellement
MicheleC a fusionné 1 révision(s) à partir de issue/38/knotes vers master il y a 4 ans
Propriétaire

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 a ajouté l'étiquette PR/rfc il y a 4 ans
MicheleC a ajouté cela au jalon R14.1.0 release il y a 4 ans
Éditeur
Propriétaire

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 a modifié le titre de Fixed incorrect update of KNote "last modified" field caused by the lauch of KNotes. à WIP: Fixed incorrect update of KNote "last modified" field caused by the lauch of KNotes. il y a 4 ans
Collaborateur

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.
Éditeur
Propriétaire

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)
Éditeur
Propriétaire

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 a modifié le titre de WIP: Fixed incorrect update of KNote "last modified" field caused by the lauch of KNotes. à Fixed incorrect update of KNote "last modified" field caused by the lauch of KNotes. il y a 4 ans
Collaborateur

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
Éditeur
Propriétaire

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.
Collaborateur

Hi Michele,

yes, tests were good.

Hi Michele, yes, tests were good.
Propriétaire

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 a fermé cette pull request il y a 4 ans
MicheleC a supprimé la branche issue/38/knotes il y a 4 ans
MicheleC a supprimé l'étiquette PR/rfc il y a 4 ans
MicheleC a modifié le jalon de R14.1.0 release à R14.0.8 release il y a 4 ans
Éditeur
Propriétaire

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

Thanks Emanoil and Slavek. Merged and backported to R14.0.8 too.
La demande d'ajout a été fusionnée manuellement en tant que d1a4daaee6.
Connectez-vous pour rejoindre cette conversation.
Aucune évaluation
Aucun jalon
Pas d'assignataires
3 participants
Notifications
Échéance

Aucune échéance n'a été définie.

Dépendances

No dependencies set.

Reference: TDE/tdepim#40
Chargement…
Il n'existe pas encore de contenu.