Skip to content

[ZEPPELIN-1290] Refactor Navbar Controller - #1281

Closed
corneadoug wants to merge 4 commits into
apache:masterfrom
corneadoug:Refactor/navbarCtrl
Closed

[ZEPPELIN-1290] Refactor Navbar Controller#1281
corneadoug wants to merge 4 commits into
apache:masterfrom
corneadoug:Refactor/navbarCtrl

Conversation

@corneadoug

Copy link
Copy Markdown
Contributor

What is this PR for?

This is a small refactoring to keep this Controller following the ControllerAs with vm
rules, that it was based on.

Here is a list of things that were changed and why:

  • Most of the controller's $scope values & fct (except from the search q) where moved to the vm.The controller is using vm, so storing in $scope to share with the view is not needed.
  • Functions or Vars that are not used in the view were removed from the vm. (kept private to the controller)
  • $rootscope functions was moved to app.js. I think the need for that function might need to be changed, but for the scope of this PR, we are just moving it to where the $rootScope values are declared.
  • Gathering vm declaration before the functions and ordered alphabetically
  • Re-order functions alphabetically
  • Create initController to regroup all the controller setup.

What type of PR is it?

Refactoring

What is the Jira issue?

https://issues.apache.org/jira/browse/ZEPPELIN-1290

How should this be tested?

You can Just verify that the below Navbar related features are still good:

  • Search Form
  • Connected Status
  • Login button
  • User Name and its dropdown menu
  • Notebook list drop-down menu (and filer, folder inside of it)

Questions:

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

@corneadoug

Copy link
Copy Markdown
ContributorAuthor

Re-trigger CI

@corneadougcorneadoug reopened this Aug 5, 2016
@corneadoug

Copy link
Copy Markdown
ContributorAuthor

@prabhjyotsingh I have Selenium test testGroupPermission failing on this PR, but can't find why.
Could you take a look since you made that test?

@prabhjyotsingh

Copy link
Copy Markdown
Contributor

Sure, let me take a look at it.

@prabhjyotsingh

Copy link
Copy Markdown
Contributor

LGTM! CI is also green.

@corneadoug

Copy link
Copy Markdown
ContributorAuthor

Awesome, Merging if there is no more discussions

@asfgitasfgit closed this in 3a1ab28Aug 9, 2016
@corneadoug
corneadoug deleted the Refactor/navbarCtrl branch August 24, 2016 08:23
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@corneadoug@prabhjyotsingh