UX improvements to transaction commits and concept deletion #178
Conversation
alexjpwalker
changed the title
TypeDBConsole.java
Outdated
| @@ -309,6 +313,7 @@ private boolean transactionREPL(TypeDBClient client, String database, TypeDBSess | |||
| break; | |||
| } else if (replCommand.isSource()) { | |||
| runSource(tx, replCommand.asSource().file(), replCommand.asSource().printAnswers()); | |||
| hasUncommittedChanges = true; | |||
There was a problem hiding this comment.
eh? what if the file only has match queries, then there's not uncommitted right?
There was a problem hiding this comment.
Yeah, I'd assumed it was hard to verify that condition, but on second inspection we can do it.
TypeDBConsole.java
Outdated
| answerCount.getAndIncrement(); | ||
| printer.conceptMap(x, tx); |
There was a problem hiding this comment.
I think this is odd - we've introduce an strong inconsistency with our protocol, rather than a minor one, by printing out all the maps that are being deleted. The user doesn't really have any interested in the concepts from which is deleted, so we shouldn't print them (they may not exist in a bit anyway!). Using this to count is ok though.
There was a problem hiding this comment.
I suppose that's fine. Seems odd to do all the work just to throw it away in the end, but given that it's not in line with the eventual solution we're aiming for, we probably shouldn't introduce unrealistic user expectations by printing them now.
TypeDBConsole.java
Outdated
| Stream<ConceptMap> matchResult = tx.query().match(query.asUpdate().match()); | ||
| AtomicInteger answerCount = new AtomicInteger(); | ||
| printCancellableResult(matchResult, x -> { | ||
| answerCount.getAndIncrement(); | ||
| printer.conceptMap(x, tx); | ||
| }); | ||
| Stream<ConceptMap> result = tx.query().update(query.asUpdate()); | ||
| printCancellableResult(result, x -> printer.conceptMap(x, tx)); | ||
| if (answerCount.get() > 0) hasUncommittedChanges = true; |
There was a problem hiding this comment.
aren't we going to print everything twice now?
TypeDBConsole.java
Outdated
| }); | ||
| if (answerCount.get() > 0) { | ||
| tx.query().delete(query.asDelete()).get(); | ||
| printer.info("Concepts have been deleted"); |
There was a problem hiding this comment.
I think you should just change this to Deleted from <count> matched answers
| @@ -0,0 +1,23 @@ | |||
| package com.vaticle.typedb.console; | |||
What is the goal of this PR?
We've added an indicator for uncommitted changes within a transaction. While you are in a transaction that has uncommitted changes, an asterisk (*) will now be displayed in the prompt.
Also, concept deletion is now more transparent; the number of concepts being deleted (or updated) is printed to the console now. This is particularly helpful for catching when there is a bug in the match query that might cause it to delete no concepts, or the wrong concepts.
What are the changes implemented in this PR?
We chose to not add a confirmation prompt prior to deletion just yet, this is due to Console architecture making that too difficult for now. This is (still!) tracked in: