ugurdogrusoz / visuall

Visuall: A tool for convenient construction of a web based visual analysis component
2 stars 0 forks source link

Unstable neo4j update #388

Closed LaraMerdol closed 1 year ago

LaraMerdol commented 1 year ago

I have made changes to the visual application in response to the issues mentioned in #387. The modifications involve using Neo4j version 5.10. Here are the details of the changes made:

  1. The APOC procedure 'apoc.cypher.runTimeboxed' has been deprecated in Neo4j 4 and later versions. Since there is no direct replacement with a built-in timeout parameter in the newer versions of APOC, I have used 'apoc.cypher.run' and implemented a timeout mechanism inside the 'neo4j-db.service.ts'.
  2. The function ID() is deprecated and it is recommended to use elementID() instead. To solve this problem, I have used elementId instead of id. The elementID is a string type and is structured as "4:c0a65d96-4993-4b0c-b036-e7ebd9174905:0". However, this type of id is not a proper selector id because it is also used as a CSS selector. To overcome this problem, I converted the colon (:) to an underscore (_) to create a valid CSS selector character sequence. For example, "4:c0a65d96-4993-4b0c-b036-e7ebd9174905:0" becomes "n4_c0a65d96-4993-4b0c-b036-e7ebd9174905_0".
  3. The default minimum password length is 8 characters, which is a requirement introduced in Neo4j 5.3.

For testing on your local environment, you can use the new Neo4j 5.10 database working on the ivis server port:3001. The credentials are username: neo4j and password: 12345678. You should change the environment.ts accordingly.

canbax commented 1 year ago

However, this type of id is not a proper selector id because it is also used as a CSS selector

You mean it is not a valid cytoscape.js selector. Right? If so where does these selectors are used? As far as I remember, they are only used in highlighting elements on hover. If it's the case, why don't we select them with selectors like [id = 'foo'] as stated https://js.cytoscape.org/#selectors/group-class-amp-id

Always replacing elementIds seems high maintenance to me. Also I see that elementId is just a string and it can be a cytoscape.js element id.

LaraMerdol commented 1 year ago

However, this type of id is not a proper selector id because it is also used as a CSS selector

You mean it is not a valid cytoscape.js selector. Right? If so where does these selectors are used? As far as I remember, they are only used in highlighting elements on hover. If it's the case, why don't we select them with selectors like [id = 'foo'] as stated https://js.cytoscape.org/#selectors/group-class-amp-id

Always replacing elementIds seems high maintenance to me. Also I see that elementId is just a string and it can be a cytoscape.js element id.

Yes, you are right. I changed the selectors as you suggested. Now all elementIds are in the same format without any replacements used.

LaraMerdol commented 1 year ago

1-) In some places, there might be some performance regression. O(n) is replaced with O(n^2). I think those can be done in O(n)

2-) I don't have access to deployed visual env. Please test there

3-) some time ago, there were Cypress e2e tests. Do they still run? If so, can you run them

4-) As an improvement those Cypress test could be run as a new step of the current Github Action

If you do points 1 & 2 I think it's good to merge. 3 & 4 could be future improvements. But if you could do point 3, it would be great to see this big change doesn't cause any bugs.

I'd like to inform you of the following updates:

  1. I've come up with an alternative solution that can enhance the run time to O(n).
  2. I've tested on my local environment. If you want to test it on your local environment, you can use the new Neo4j 5.10 database running on the ivis server port:3001. The credentials to log in are username: neo4j and password: 12345678. You should also update the environment.ts file accordingly.
  3. Unfortunately, I'm unable to run Cypress e2e tests as it keeps giving me a 503: Service Unavailable error.