[ZEPPELIN-2301] DON'T overwrite editor text when note is renamed, created, ...

[1ambda]

What is this PR for?

DON'T overwrite local text change in editor when note is renamed, created, and so on

• Move up paragraph and Move down paragraph and other actions calling saveNote will reset local text change. But I will resolve it in different PRs.

Implementation Details

We can't access to children's $scope in angular. That's the reason why paragraphText is written as a (global) service.

What type of PR is it?

[Bug Fix]

Todos

NONE

What is the Jira issue?

ZEPPELIN-2301

Related with

• [ZEPPELIN-1492] fixing the issue where updating a paragraph was not propagated correctly #2120 (different approach)
• https://issues.apache.org/jira/browse/ZEPPELIN-1492 (people are complaining on this issue. but different topic.)

How should this be tested?

1. Open the same note in 2 browsers (use different sessions e.g normal chrome + secret chrome or firefox + safari)
2. Prepare a paragraph synchronized among sessions.
3. Modify text in one browser.
4. (Create, Delete) Rename other notes. The actions which triggering NOTE websocket messages should not overwrite local text change in editor.

Screenshots (if appropriate)

After

Questions:

• Does the licenses files need update? - NO
• Is there breaking changes for older versions? - NO
• Does this needs documentation? - NO

[FRosner]

I am not sure I understand the exact difference between this and #2176. Can you try to upload bigger versions of the gif or elaborate a bit on the difference? Are the two PR (#2176 and #2177) fixes for the same problem but in a different way or are they addressing different problems?

[1ambda]

@FRosner I updated GIF.

What #2176 and #2177 is trying to prevent local change from overwritten. Overwriting occurs in many cases.

• [ZEPPELIN-2300] SHOULD NOT update paragraph text when local change exists #2176 is only about when other browsers run the same paragraph
• [ZEPPELIN-2301] DON'T overwrite editor text when note is renamed, created, ... #2177 is related with the case when people rename, remove, create notes
• (not resolved because it requires more big modification) Additionally, move up and move down also reset local changes.

Here is another opinion about overwriting

Right, overwrite text and losing local edit is bad experience. (ZEPPELIN-2246)

[FRosner]

Gotcha @1ambda,

The gifs make it clear now, thanks! Not sure about the exact code changes as I'm not that familiar with the code base you're touching. Do you have any specific part I should dive into or can someone else also take a look?

I agree with the priorities as well. If we combine it with save-on-lost-focus the time window is at least lower to get your changes overwritten.

Just one more related question: I noticed that there is a save timer for notebooks in the code. I tried to figure out when it is triggered but I didn't succeed. What is it used for? When is it triggered? What's the difference between saveNote and saving of a paragraph?

Thanks!
Frank

[1ambda]

@FRosner Anything would be greatly appreciated. For example trying this PR, suggestion for how to write test code for this, and so on.

regarding to triggering auto-save, Zeppelin sends the startSaveTimer broadcast message, but there is no $scope.$on('startSaveTimer', ...). That's the reason why auto save is not working properly.

// app/notebook/paragraph/paragraph.controller.js#L598

  $scope.aceChanged = function(_, editor) {
    var session = editor.getSession();
    var dirtyText = session.getValue();
    $scope.dirtyText = dirtyText;
    $scope.$broadcast('startSaveTimer');
    setParagraphMode(session, dirtyText, editor.getCursorPosition());
  };

[1ambda]

@AhyoungRyu Could you help review?

[FRosner]

Thanks @1ambda. I'm quite busy at this moment so if someone else (e.g. @AhyoungRyu) can also take a look that would be greatly appreciated.

[AhyoungRyu]

Sure. I'll test it and get back you soon.

[1ambda]

Let me implement locking and realtime notification soon. I will close this PR.

• https://issues.apache.org/jira/browse/ZEPPELIN-2340