Handle images with no user properly (#147)

* Uncomment test for no repositoryUser in `app/app.spec.js`, so I can work on getting this case working. * Add $log to repository-detail-controller * Set $scope.repository properly even if no user Check if `$scope.repositoryUser` is defined and if it's not, set `$scope.repository` simply to `$scope.repositoryName`. * Add route for repo with no repositoryUser * Make tagsPerPage dropdown use query string vars instead of putting `tagsPerPage` in the route part of the URL, which makes it difficult to distinguish between `repositoryName` and `tagsPerPage`, because the routes are too similar. * Make first/next links use query string vars instead of putting `tagsPerPage` in the route part of the URL, which makes it difficult to distinguish between `repositoryName` and `tagsPerPage`, because the routes are too similar. * Move defaultTagsPerPage from `RepositoryListController` to `TagController`. I think it makes more sense to read `defaultTagsPerPage` in `TagController` than to do it in `RepositoryListController` and have to pass it in URLs. * Fix "Details for repository" for repo w/ no user * Fix breadcrumb when no repositoryUser * Remove tagsPerPage/tagPage from repo detail routes * Update unit tests to use tagsPerPage qs param Updated the unit tests to page `tagsPerPage` as a query string parameter rather than in the route. * Uncomment assertions about scope.reposPerPage an make them work by casting string to an int with `parseInt`. * Make /tag work for image with no repositoryUser * repository-detail.html: Fix breadcrumb link for case when there is no `repositoryUser`. * tag-detail.html: Fix breadcrumb link when image has no `repositoryUser`. * tag-detail.html: Fix shown docker pull command when image has no `repositoryUser`. * tag-detail.html: Show docker pull cmd in code font I think it looks better.
kwk · Aug 4, 2016 · 3ad864b · 3ad864b
1 parent c34f619
commit 3ad864b
Show file tree

Hide file tree

Showing 8 changed files with 79 additions and 64 deletions.
diff --git a/app/app.js b/app/app.js
@@ -62,9 +62,9 @@ angular
       }).
       when('/repository/:repositoryUser/:repositoryName', {
         templateUrl: 'repository/repository-detail.html',
-        controller: 'RepositoryDetailController',
+        controller: 'RepositoryDetailController'
       }).
-      when('/repository/:repositoryUser/:repositoryName/:tagsPerPage?/:tagPage?', {
+      when('/repository/:repositoryName', {
         templateUrl: 'repository/repository-detail.html',
         controller: 'RepositoryDetailController'
       }).
@@ -75,7 +75,7 @@ angular
 	    when('/about', {
         templateUrl: 'about.html',
       }).
-      when('/tag/:repositoryUser/:repositoryName/:tagName/', {
+      when('/tag/:repositoryUser?/:repositoryName/:tagName/', {
         templateUrl: 'tag/tag-detail.html',
         controller: 'TagController',
       }).

diff --git a/app/app.spec.js b/app/app.spec.js
@@ -50,7 +50,7 @@ describe('docker-registry-frontend', function() {
     expect($route.current.controller).toBe('RepositoryListController');
     var scope = {};
     $controller('RepositoryListController', {$scope: scope});
-    // expect(scope.reposPerPage).toBe(10);
+    expect(scope.reposPerPage).toBe(10);
   });
 
   it('/repositories/20 should display repository list page', function() {
@@ -61,7 +61,7 @@ describe('docker-registry-frontend', function() {
     expect($route.current.controller).toBe('RepositoryListController');
     var scope = {};
     $controller('RepositoryListController', {$scope: scope});
-    // expect(scope.reposPerPage).toBe(20);
+    expect(scope.reposPerPage).toBe(20);
   });
 
   it('URL with repositoryUser and repositoryName and no tagsPerPage should display repository detail page', function() {
@@ -80,7 +80,8 @@ describe('docker-registry-frontend', function() {
 
   it('URL with repositoryUser and repositoryName and tagsPerPage should display repository detail page', function() {
     $httpBackend.expectGET('repository/repository-detail.html').respond(200);
-    $location.path('/repository/owner/name/10');
+    $location.path('/repository/owner/name');
+    $location.search('tagsPerPage', 10)
     $rootScope.$digest();
     expect($route.current.templateUrl).toBe('repository/repository-detail.html');
     expect($route.current.controller).toBe('RepositoryDetailController');
@@ -91,28 +92,26 @@ describe('docker-registry-frontend', function() {
     expect(scope.repository).toBe('owner/name');
   });
 
-  // This test currently fails; this URL is incorrectly routing to the home page
-  // @todo @FIXME
-  //
-  // it('URL with repositoryName but no repositoryUser and no tagsPerPage should display repository detail page', function() {
-  //   $httpBackend.expectGET('repository/repository-detail.html').respond(200);
-  //   $location.path('/repository/cx');
-  //   $rootScope.$digest();
-  //   expect($route.current.templateUrl).toBe('repository/repository-detail.html');
-  //   expect($route.current.controller).toBe('RepositoryDetailController');
-  // });
+  it('URL with repositoryName but no repositoryUser and no tagsPerPage should display repository detail page', function() {
+    $httpBackend.expectGET('repository/repository-detail.html').respond(200);
+    $location.path('/repository/cx');
+    $rootScope.$digest();
+    expect($route.current.templateUrl).toBe('repository/repository-detail.html');
+    expect($route.current.controller).toBe('RepositoryDetailController');
+  });
 
   it('URL with repositoryName but no repositoryUser and tagsPerPage should display repository detail page', function() {
     $httpBackend.expectGET('repository/repository-detail.html').respond(200);
-    $location.path('/repository/cx/10');
+    $location.path('/repository/cx');
+    $location.search('tagsPerPage', 10)
     $rootScope.$digest();
     expect($route.current.templateUrl).toBe('repository/repository-detail.html');
     expect($route.current.controller).toBe('RepositoryDetailController');
     var scope = {};
     $controller('RepositoryDetailController', {$scope: scope});
-    // expect(scope.repositoryUser).toBeUndefined();
-    // expect(scope.repositoryName).toBe('cx');
-    // expect(scope.repository).toBe('cx');
+    expect(scope.repositoryUser).toBeUndefined();
+    expect(scope.repositoryName).toBe('cx');
+    expect(scope.repository).toBe('cx');
   });
 
   it('/about should display about page', function() {
@@ -136,16 +135,13 @@ describe('docker-registry-frontend', function() {
     expect(scope.tagName).toBe('latest');
   });
 
-  // This test currently fails; this URL is incorrectly routing to the home page
-  // @todo @FIXME
-  //
-  // it('/tag/repositoryName/latest should display tag detail page', function() {
-  //   $httpBackend.expectGET('tag/tag-detail.html').respond(200);
-  //   $location.path('/tag/repositoryName/latest');
-  //   $rootScope.$digest();
-  //   expect($route.current.templateUrl).toBe('tag/tag-detail.html');
-  //   expect($route.current.controller).toBe('TagController');
-  // });
+  it('/tag/repositoryName/latest should display tag detail page', function() {
+    $httpBackend.expectGET('tag/tag-detail.html').respond(200);
+    $location.path('/tag/repositoryName/latest');
+    $rootScope.$digest();
+    expect($route.current.templateUrl).toBe('tag/tag-detail.html');
+    expect($route.current.controller).toBe('TagController');
+  });
 
   it('/image/88e37c7099fa should display image detail page', function() {
     $httpBackend.expectGET('tag/image-detail.html').respond(200);

diff --git a/app/repository/repository-detail-controller.js b/app/repository/repository-detail-controller.js
@@ -8,8 +8,8 @@
  * Controller of the docker-registry-frontend
  */
 angular.module('repository-detail-controller', ['registry-services', 'app-mode-services'])
-  .controller('RepositoryDetailController', ['$scope', '$route', '$routeParams', '$location', '$modal', 'Repository', 'AppMode',
-  function($scope, $route, $routeParams, $location, $modal, Repository, AppMode){
+  .controller('RepositoryDetailController', ['$scope', '$route', '$routeParams', '$location', '$log', '$modal', 'Repository', 'AppMode',
+  function($scope, $route, $routeParams, $location, $log, $modal, Repository, AppMode){
 
     $scope.$route = $route;
     $scope.$location = $location;
@@ -18,7 +18,14 @@ angular.module('repository-detail-controller', ['registry-services', 'app-mode-s
     //$scope.searchTerm = $route.current.params.searchTerm;
     $scope.repositoryUser = $route.current.params.repositoryUser;
     $scope.repositoryName = $route.current.params.repositoryName;
-    $scope.repository = $scope.repositoryUser + '/' + $scope.repositoryName;
+    $log.log('repository-detail-controller: $scope.repositoryUser = ' + $scope.repositoryUser);
+    if ($scope.repositoryUser == null || $scope.repositoryUser == 'undefined') {
+      $scope.repository = $scope.repositoryName;
+      $log.log('repository-detail-controller: $scope.repositoryUser was undefined; setting repository to just repositoryName = ' + $scope.repository);
+    } else {
+      $scope.repository = $scope.repositoryUser + '/' + $scope.repositoryName;
+      $log.log('repository-detail-controller: $scope.repositoryUser was NOT undefined; setting repository to ' + $scope.repository);
+    }
 
     $scope.appMode = AppMode.query();
     $scope.maxTagsPage = undefined;
@@ -27,13 +34,13 @@ angular.module('repository-detail-controller', ['registry-services', 'app-mode-s
     $scope.getNextHref = function (){
       if($scope.maxTagsPage > $scope.tagsCurrentPage){
         var nextPageNumber = $scope.tagsCurrentPage + 1;
-        return '/repository/'+$scope.repository+'/'+ $scope.tagsPerPage +'/' +nextPageNumber;
+        return '/repository/'+$scope.repository+'?tagsPerPage='+ $scope.tagsPerPage +'&tagPage=' +nextPageNumber;
       }
       return '#'
-    } 
+    }
     $scope.getFirstHref = function (){
       if($scope.tagsCurrentPage > 1){
-        return '/repository/'+$scope.repository+'/' + $scope.tagsPerPage +'/1';
+        return '/repository/'+$scope.repository+'?tagsPerPage=' + $scope.tagsPerPage;
       }
       return '#'
     }

diff --git a/app/repository/repository-detail.html b/app/repository/repository-detail.html
@@ -1,13 +1,12 @@
 <ol class="breadcrumb">
     <li><a href="home">Home</a></li>
     <li><a href="repositories/">Repositories</a></li>
-    <li><a href="repositories/{{repositoryUser}}">{{repositoryUser}}</a></li>
-    <li><a href="repository/{{repositoryUser}}/{{repositoryName}}">{{repositoryName}}</a></li>
+    <li ng-show="repositoryUser"><a href="repositories/{{repositoryUser}}">{{repositoryUser}}</a></li>
+    <li><a href="repository/{{repository}}">{{repositoryName}}</a></li>
 </ol>
 
 <h1>
-  Details for repository
-  <a href="repositories/{{repositoryUser}}">{{repositoryUser}}</a>/<a href="repository/{{repositoryUser}}/{{repositoryName}}">{{repositoryName}}</a>
+  Details for repository: {{repository}}
   <div ng-hide="appMode.browseOnly" class="pull-right">
     <!-- Maybe put this into its own directive? -->
     <button type="button" ng-click="selectedRepositories=['{{repositoryUser}}/{{repositoryName}}']; openConfirmRepoDeletionDialog()" class="btn btn-danger" ng-hide="appMode.browseOnly">
@@ -40,14 +39,14 @@ <h1>
           <span class="caret"></span>
         </a>
         <ul class="dropdown-menu">
-          <li><a href="repository/{{repository}}">Show all</a></li>
+          <li><a href="repository/{{repository}}?tagsPerPage=all">Show all</a></li>
           <li role="separator" class="divider"></li>
-          <li><a href="repository/{{repository}}/10/1">10</a></li>
-          <li><a href="repository/{{repository}}/20/1">20</a></li>
-          <li><a href="repository/{{repository}}/40/1">40</a></li>
-          <li><a href="repository/{{repository}}/60/1">60</a></li>
-          <li><a href="repository/{{repository}}/80/1">80</a></li>
-          <li><a href="repository/{{repository}}/100/1">100</a></li>
+          <li><a href="repository/{{repository}}?tagsPerPage=10">10</a></li>
+          <li><a href="repository/{{repository}}?tagsPerPage=20">20</a></li>
+          <li><a href="repository/{{repository}}?tagsPerPage=40">40</a></li>
+          <li><a href="repository/{{repository}}?tagsPerPage=60">60</a></li>
+          <li><a href="repository/{{repository}}?tagsPerPage=80">80</a></li>
+          <li><a href="repository/{{repository}}?tagsPerPage=100">100</a></li>
         </ul>
       </div>
     </li>

diff --git a/app/repository/repository-list-controller.js b/app/repository/repository-list-controller.js
@@ -19,11 +19,13 @@ angular.module('repository-list-controller', ['ngRoute', 'ui.bootstrap', 'regist
     $scope.repositoryName = $route.current.params.repositoryName;
     $scope.repository = $scope.repositoryUser + '/' + $scope.repositoryName;
 
-    $scope.appMode = AppMode.query( function (result){
-      $scope.defaultTagsPerPage = result.defaultTagsPerPage
+    AppMode.query(function(result) {
+      $scope.appMode = result;
     });
     // How to query the repository
-    $scope.reposPerPage = $route.current.params.reposPerPage;
+    if ($route.current.params.reposPerPage) {
+      $scope.reposPerPage = parseInt($route.current.params.reposPerPage, 10);
+    }
     $scope.lastNamespace = $route.current.params.lastNamespace;
     $scope.lastRepository = $route.current.params.lastRepository;
     var queryObject = {};

diff --git a/app/repository/repository-list.html b/app/repository/repository-list.html
@@ -38,7 +38,7 @@ <h1>Repositories</h1>
           <!--<input type="checkbox" name="selectedRepos[]" value="{{repo.name}}" ng-model="repo.selected" ng-hide="appMode.browseOnly">-->
         </td>
         <td class="grow">
-          <a href="repository/{{repo.name}}/{{defaultTagsPerPage}}"><!--<span class="glyphicon glyphicon-book"></span>--> {{repo.name|trim:username+'/'}}</a>
+          <a href="repository/{{repo.name}}"><!--<span class="glyphicon glyphicon-book"></span>--> {{repo.name|trim:username+'/'}}</a>
         </td>
       </tr>
     </tbody>

diff --git a/app/tag/tag-controller.js b/app/tag/tag-controller.js
@@ -8,8 +8,8 @@
  * Controller of the docker-registry-frontend
  */
 angular.module('tag-controller', ['registry-services'])
-  .controller('TagController', ['$scope', '$route', '$routeParams', '$location', '$log', '$filter', 'Manifest', 'Tag', 'filterFilter', '$modal',
-  function($scope, $route, $routeParams, $location, $log, $filter, Manifest, Tag, filterFilter, $modal){
+  .controller('TagController', ['$scope', '$route', '$routeParams', '$location', '$filter', 'Manifest', 'Tag', 'AppMode', 'filterFilter', '$modal',
+  function($scope, $route, $routeParams, $location, $filter, Manifest, Tag, AppMode, filterFilter, $modal){
 
     $scope.$route = $route;
     $scope.$location = $location;
@@ -18,9 +18,20 @@ angular.module('tag-controller', ['registry-services'])
     $scope.searchName = $route.current.params.searchName;
     $scope.repositoryUser = $route.current.params.repositoryUser;
     $scope.repositoryName = $route.current.params.repositoryName;
-    $scope.repository = $scope.repositoryUser + '/' + $scope.repositoryName;
+    if ($scope.repositoryUser == null || $scope.repositoryUser == 'undefined') {
+      $scope.repository = $scope.repositoryName;
+    } else {
+      $scope.repository = $scope.repositoryUser + '/' + $scope.repositoryName;
+    }
     $scope.tagName = $route.current.params.tagName;
-    $scope.tagsPerPage = $route.current.params.tagsPerPage;
+    AppMode.query(function(result) {
+      $scope.appMode = result;
+      console.log('$route.current.params.tagsPerPage = ' + $route.current.params.tagsPerPage);
+      $scope.tagsPerPage = $route.current.params.tagsPerPage || $scope.appMode.defaultTagsPerPage;
+      if ($scope.tagsPerPage == 'all') {
+        $scope.tagsPerPage = null;
+      }
+    });
 
     // Fetch tags
     $scope.tags = Tag.query({
@@ -94,4 +105,4 @@ angular.module('tag-controller', ['registry-services'])
       });
     };
 
-  }]);
+  }]);
diff --git a/app/tag/tag-detail.html b/app/tag/tag-detail.html
@@ -1,17 +1,17 @@
 <ol class="breadcrumb">
     <li><a href="home">Home</a></li>
     <li><a href="repositories/">Repositories</a></li>
-    <li><a href="repositories/{{repositoryUser}}">{{repositoryUser}}</a></li>
-    <li><a href="repository/{{repositoryUser}}/{{repositoryName}}">{{repositoryName}}</a></li>
-    <li><a href="tag/{{repositoryUser}}/{{repositoryName}}/{{tagName}}/{{imageId}}">{{tagName}}</a></li>
+    <li ng-show="repositoryUser"><a href="repositories/{{repositoryUser}}">{{repositoryUser}}</a></li>
+    <li><a href="repository/{{repository}}">{{repositoryName}}</a></li>
+    <li><a href="tag/{{repository}}/{{tagName}}/{{imageId}}">{{tagName}}</a></li>
 </ol>
 
 <h1>
-  Details for tag 
+  Details for tag
   <a href="repositories/{{repositoryUser}}">{{repositoryUser}}</a> /
   <a href="repository/{{repositoryUser}}/{{repositoryName}}">{{repositoryName}}</a> :
   <a href="tag/{{repositoryUser}}/{{repositoryName}}/{{tagName}}/{{imageId}}">{{tagName}}</a>
-  
+
   <div ng-hide="appMode.browseOnly" class="pull-right">
     <button type="button" ng-click="selection=['{{repositoryUser}}/{{repositoryName}}:{{tagName}}']; openConfirmTagDeletionDialog()" class="btn btn-danger">
       <span class="glyphicon glyphicon-trash"></span> Delete Tag
@@ -26,8 +26,8 @@ <h1>
 
 </h1>
 
-<div class="well">
-  docker pull {{registryHost.host}}<span ng-if="registryHost.port != 80 && registryHost.port != 443">:{{registryHost.port}}</span>/{{repositoryUser}}/{{repositoryName}}:{{tagName}}
-</div>
+<div class="well"><code>
+  docker pull {{registryHost.host}}<span ng-if="registryHost.port != 80 && registryHost.port != 443">:{{registryHost.port}}</span>/{{repository}}:{{tagName}}
+</code></div>
 
 <image-details></image-details>