Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show hint about :auto on CALL {...} IN TRANSACTIONS #1637

Merged
merged 6 commits into from Jan 27, 2022

Conversation

OskarDamkjaer
Copy link
Contributor

@OskarDamkjaer OskarDamkjaer commented Jan 10, 2022

We do a few regex checks on error messages to see if the error was caused by a missing :auto prefix, if so we add a hint to the error message.

This PR adds a check for if the error is the new CALL IN {} TRANSACTIONS which needs :auto and updates the check for PERIODIC COMMIT (that has the new error message"A query with 'PERIODIC COMMIT' can only be executed in an implicit transaction but tried to execute in an explicit transaction.").

I also add a few tests to verify the behaviour, updated the :help auto to use the new syntax and added a helper method to cypress for faster tests.

CleanShot 2022-01-10 at 14 41 32

URL for testing: update_warning.surge.sh
Do note that a 4.4 db is needed.

@OskarDamkjaer OskarDamkjaer changed the title Update warning Show hint about :auto on CALL {...} IN TRANSACTIONS Jan 10, 2022
@OskarDamkjaer OskarDamkjaer removed the request for review from eijawerner January 17, 2022 16:27

it('adding :auto enables running periodic commit', () => {
cy.executeCommand(':auto USING PERIODIC COMMIT RETURN "Laverre";')
cy.getFrames().contains('ERROR')
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment explaining why we're looking for an error here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure, it definitely looks a bit odd

Copy link
Member

@nglgzz nglgzz left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍

@OskarDamkjaer OskarDamkjaer merged commit 9ef5d1b into neo4j:master Jan 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants