fix: Fix documents losing associated comments when changing storage path#349
fix: Fix documents losing associated comments when changing storage path#349
Conversation
|
Hello @pauliyobo You wrote:
Is it possible for you to put the ruff lintting changes in a separate PR?
I think giving dialog might be better, but I haven't tested this yet, I hope to test this soon, it's so cool. |
probably, though I accidentally ran format before committing the prior change, which makes things a bit complicated. |
328300a to
3aa86eb
Compare
3aa86eb to
328300a
Compare
|
Hello @pauliyobo |
|
Hello |
|
Thanks @pauliyobo |
|
@cary-rowen apologies for the delay. |
|
Hi @pauliyobo Same error: |
|
@cary-rowen |
|
@pauliyobo |
|
Even after solving the above problems, I have discovered two other noteworthy issues:
|
|
@cary-rowen |
|
Apologies for the double comment. How likely is it that an user will update their version, to then run earlier versions against the same database? Perhaps we could offer a mechanism to downgrade the database version from the latest version in order to return to the previous revision, but I don't know if it's worth the effort. |
|
Regarding the first point, is it possible for us to design a migration dialog, even if it's just for showing progress, instead of working completely silently in the background, which would give the user the illusion that the software isn't running properly. |
|
On the second point, I don't think downgrading is a use case that should be supported. |
|
Hey @pauliyobo I'm sorry to have interfered with your work. |
Co-authored-by: wencong <manchen_0528@outlook.com>
|
@cary-rowen |
|
I guess one other way could also be to just run the migration with only the schema changes and allow the user to manually populate the content hashes from the app itself. That should solve the long migration time and the progress issues. |
|
That sounds like a good plan. We could run the schema migration at startup but defer the hash calculation until the user opens a specific document. |
|
Sorry, perhaps I was unclear. |
|
I understand. However, I suggest that we implement a popup dialog to ask the user if they want to perform this manual migration (e.g., after the update). This would greatly improve the feature's discoverability, ensuring users know they have the option to update everything at once. Or, do you have other thoughts? |
Conflicts resolved by moving 'blake3' dependency from requirements-app.txt to pyproject.toml.
|
Looking forward to your further updates @pauliyobo. Also, is it possible for us to release a new version in the near future? I'd like to consolidate database migration work into a single version. |
|
Actually I'm really looking forward to this, if you need me to do anything, please let me know. |
Link to issue number:
closes #229
Summary of the issue:
Up until now, annotations for a document were associated only through the document's title and storage URI.
While this has worked for most cases, changing the location of the original document would consequently invalidate the association, even though the annotations and the document in question would not have any modification.
Description of how this pull request fixes the issue:
This PR allows documents to be found also through their content hash.
This will cover cases in which the storage location has changed, however, it will not cover the cases in which the content of a document has been modified, though if the storage location remains the same, there shouldn't be any changes i nthe behaviour.
Testing performed:
Manual and unit testing
Known issues with pull request:
While this isn't really an issue, I fear that the migration may take a significant amount of time for large collections. It would need to be tested.
PS: Sorry for the horrible review experience,
ruff formattouched way more than I'd have expected.@cary-rowen I believe you were particularly affected by this issue. Any thoughts?
Also, I wonder if the user should be prompted whenever the storage location of the document differs from the actual path stored in the database.