Skip to content

Conversation

tdomhan
Copy link
Contributor

@tdomhan tdomhan commented Jan 11, 2024

This is an attempt at a fix. There may be a better way to do this. When cutting + pasting the root node's references are not preserved. The problem is that the root-block returned by db/pull misses the :block/_refs while blocks returned by db/get-block-and-children contain :block/_refs. I know too little about the inner workings for Logseq and Datascript to know if this is expected. If someone could let me know whether or not this is expected or whether there is a way to modify db/pull to return :block/_refs that would be great.

More details can be found in this issue: #4491

@andelf andelf self-requested a review January 11, 2024 06:55
@andelf
Copy link
Collaborator

andelf commented Jan 11, 2024

Sorry. I cannot reproduce the original bug.
Could you be more clear about what the original bug is?
And the precise steps to reproduce it.

Screen.Recording.2024-01-11.at.4.03.13.PM.mov

The above is my steps to reproduce the behavior. Am I missing something?

@tdomhan
Copy link
Contributor Author

tdomhan commented Jan 11, 2024

Thanks for taking a look!
I tried to record a video of what I mean. The content is copied correctly and UUIDs are also correct. However for the root note that gets cut the references are no longer shown (I mean the 1 at the right of the block). After re-indexing this would appear again as the UUIDs are correct, however the :db/id doesn't get updated as somehow the :block/_refs are missing from what is returned by db/pull (not sure if this is expected or not).
The other (maybe) issue is that the references contain the old and new :db/id in their path-refs. This might not be an issue in practice, not sure.
In any case let me know if this makes sense or you'd like me to provide anything else.

Screen.Recording.2024-01-11.at.10.21.43.mov

@tdomhan
Copy link
Contributor Author

tdomhan commented Jan 14, 2024

Here are some details on the other problem:

Screen.Recording.2024-01-14.at.11.49.513.mov

After the cut the :block/ref is correctly gone. After the paste however the block will reference both the old and the new db/id of the block (so one will actually no longer exist). Do you have any idea what part of the code could cause this?

Copy link
Collaborator

@andelf andelf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@andelf andelf requested a review from tiensonqin January 15, 2024 03:39
@tdomhan
Copy link
Contributor Author

tdomhan commented Jan 15, 2024

Thanks. It would probably be also good to address the problem of the stale :db/ids in the references. Without a fix to the problem of stale, non-existing, :db/id one runs into issues when interaction with the old blocks. (I see this in the console: #error {:message "Cannot store nil as a value at [:db/add 38443 :block/path-refs nil]", :data {:value nil, :context [:db/add 38443 :block/path-refs nil], :error :transact/syntax}}).

I found the problem with the references to old db/id. They are added via revert-cut-txs. Here is what revert-cut-txs looks like:

([:db/add 85 :block/refs 90]
 [:db/add 85 :block/path-refs 90]
 [:db/add 85 :block/content "((659f6f20-868f-421b-88c0-2c9bf0346461))"]
 [:db/add 81 :block/refs 99]
 [:db/add 81 :block/path-refs 99]
 [:db/add 81 :block/content "((659f6f26-8b03-4ad8-8b6b-0205c68130f3))"]
 [:db/add 88 :block/refs 94]
 [:db/add 88 :block/path-refs 94]
 [:db/add 88 :block/content "((659f6f27-5175-4054-9d7b-56e77d3f007c))"]
 [:db/add 81 :block/refs 99]
 [:db/add 81 :block/path-refs 99]
 [:db/add 81 :block/content "((659f6f26-8b03-4ad8-8b6b-0205c68130f3))"]
 [:db/add 88 :block/refs 94]
 [:db/add 88 :block/path-refs 94]
 [:db/add 88 :block/content "((659f6f27-5175-4054-9d7b-56e77d3f007c))"]
 [:db/add 88 :block/refs 94]
 [:db/add 88 :block/path-refs 94]
 [:db/add 88 :block/content "((659f6f27-5175-4054-9d7b-56e77d3f007c))"])

For example there is :db/add 85 :block/refs 90 which refers to 90, the old :db/id that no longer exists. Removing revert-cut-txs solves the issue here. However, the revert is needed to add back the block references instead of the copied text that that was inserted in the cut. One fix could be to remove the refs and path-refs from the revert txs. Another solution would be trying to preseve the :db/id from before the cut and not create a new :db/id. I'm not sure if there's a better solution. What do you think?

@tdomhan
Copy link
Contributor Author

tdomhan commented Jan 18, 2024

I updated the PR to not include references to non-existing :db/id in revert-cut-txs. This solves the issue for the problem I was facing. Note that :db/ids are removed in outliner.core.cljs in function insert-blocks-aux.

Let me know if this solution works. I don't have a good enough overview of the code base to know whether this would break something else. If there's another test case I should try also let me know.

@tdomhan
Copy link
Contributor Author

tdomhan commented Jan 18, 2024

Here's the behavior after the changes:

  • all block references are there
  • the blocks that contain the references only point to the existing :db/ids
Screen.Recording.2024-01-18.at.08.31.28420.mov

@tdomhan
Copy link
Contributor Author

tdomhan commented Feb 8, 2024

Should we merge this or is there anything else missing?

@andelf andelf merged commit 7ac3a64 into logseq:master Feb 29, 2024
@tdomhan tdomhan deleted the fix_cut_ref branch March 6, 2024 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants