mbraak / jqTree

Tree widget for jQuery
https://mbraak.github.io/jqTree/
Apache License 2.0
1.02k stars 177 forks source link

Bug - two nodes are selected #452

Closed jcr4e closed 7 years ago

jcr4e commented 7 years ago

bug

This happens when you add and remove, and then add nodes again. This issue occurs at random, and there's no particular pattern of steps on when this happens so it's really difficult to recreate. It happened like 20 times in an hour of testing.

mbraak commented 7 years ago

That's interesting. It is possible that selection errors occurs when nodes have no id or have a duplicate id. So it could be related to that.

Do the nodes in your tree have an id?

jcr4e commented 7 years ago

Thanks for your reply. Yes, my nodes have ID. I do a console.log when I click a node and I get the IDs for the nodes that I clicked except when this error happens. One of the highlighted nodes show an ID and the other one shows "undefined".

mbraak commented 7 years ago

It's strange that a node has the id 'undefined'. It could be the cause of the error.

jcr4e commented 7 years ago

It is definitely the cause of the error, but why would a node lose an id at random when the others that I have created using the same process?

Here's how I add a child node to a parent node:

var new_id = data; // this is a dynamic value. e.g. 100 $tree.tree( 'appendNode', { name: 'New Step', id: new_id }, parent_node ); $tree.tree('openNode', parent_node); var node = $tree.tree('getNodeById', new_id); $tree.tree('selectNode', node);

Here's how I delete a node:

var $tree = $('#treeview'); var node = $tree.tree('getNodeById', step_id); $tree.tree('removeNode', node);

mbraak commented 7 years ago

I will try to reproduce the error.

mbraak commented 7 years ago

I'm afraid I cannot reproduce this error.

jcr4e commented 7 years ago

Please watch this video. Thanks :) http://screencast.com/t/z4jQrROTa

jcr4e commented 7 years ago

Well it's time for me to go home now. I would be thankful if you could help me figure this out. I'd be happy to send you parts of my code so you would have a better idea of what might be causing the issue. Thanks again for your help. Great work on jqTree by the way. I really like it.

mbraak commented 7 years ago

Thank you. I would certainly be interested to see parts of your code.

By the way, the first error message that you saw in the video is raised by the function updateNode. It is possible that you are calling updateNode with invalid arguments.

jcr4e commented 7 years ago

Hi. It's strange because I don't have a function named "updateNode". If a user keeps playing with the Add and Delete steps like I did in the video, the error will occur at random. I think the problem is that the tree is not redrawing correctly. When I "View Details" of the step that I'm trying to delete, a popup says "ERROR: This step doesn't exist anymore" (it has already been deleted from the database during the first click on "Delete this Step"). The error happens when you click "Delete this Step" again because you are trying to delete something that doesn't exist, and shouldn't be on the tree anymore. Please see attached screenshots. bug-000 bug-001 bug-002

jcr4e commented 7 years ago

Actually the step/node is still there. When I refresh the page, the node still exists and when I click "View Details", I can now see the step details. So it seems that the actual problem is about the subject node's properties "getting lost" temporarily when you repeatedly do appendNode and/or removeNode. bug-003

mbraak commented 7 years ago

I would be very interested to see parts of your source code. Perhaps that will help me to find this error. Could you please share some code?

jcr4e commented 7 years ago

Hi. I've attached a zip file containing parts of the project and also the SQL dump file. If you have a local Apache server, you can set up the project and run this page directly so you can play around with the add/remove nodes.

http://localhost/workflow/workflow/edit/3

Thanks for all your assistance.

workflow.zip

mbraak commented 7 years ago

Thanks! I will have a look.

mbraak commented 7 years ago

Unfortunately I cannot get it to run after the login page. Perhaps I'm using the wrong php version (php 7).

I did see something strange in controllers/Step.php. It says:

public function getNewStepID() {
    $last_id = $this->db->query("SELECT id FROM steps ORDER BY id DESC LIMIT 1")->row();
    //echo $last_id->id + 1;
    echo $last_id->id;
}

I think it returns an existing id. That could be an error.

jcr4e commented 7 years ago

I'm sorry, I forgot to give you a test login credential:

username: fredr@sonarlogic.biz password: 12345

This is correct: echo $last_id->id;

This is the flow I created when adding a node to the tree:

  1. The new step is inserted into the database (steps table) first
  2. New node is added into the tree, and its ID is based on that last ID inserted into the database
mbraak commented 7 years ago

I got your app running and it looks very nice.

I do think there is some logic in the workflow/edit.php view that can cause duplicate ids.

On line 258 you do a post for step/addStep:

$.ajax({
  url: '<?php echo base_url(); ?>step/addStep',
...

Then on line 277 you do a get request for the new id:

$.get("<?php echo base_url(); ?>step/getNewStepID", function(data) {
...

And add a node with this id on line 279:

$tree.tree(
    'appendNode', {
        name: 'New Step',
        id: new_id
    },
    parent_node
);

It is possible that you do the getNewStepID request before the addStep is finished, thus getting an old id.

I think you should do the appendNode in the success function of the addStep request (line 262).

I hope this helps

jcr4e commented 7 years ago

Hi, your comment totally makes sense. I think that will solve the problem. Why haven't I thought of that? Let me test this for at least 2 hours. I bet I wouldn't see that error anymore. Thanks a lot for your time looking at this issue and debugging my code. I really appreciate it.

This project is an attempt to be a single app alternative to Agileboard and Asana. jqTree is a very important part of it.

workflow-001 workflow-002 workflow-003

mbraak commented 7 years ago

I hope this works. Closing the issue.