Hide repository admin panel if no connectors enabled

[Ryan J Ollos]

The proposed change is to have the Manage Repositories admin panel visible only when at least one repository connector is enabled (comment:2:ticket:11713).

[Ryan J Ollos]

Proposed changes in log:rjollos.git:t12248_hide_repository_admin_panels.

[Ryan J Ollos]

Committed to trunk in [14684].

[Ryan J Ollos]

Functional tests are failing.

[Ryan J Ollos]

Failing functional tests fixed in [14687].

[Jun Omae]

Since [14684], repository admin page wouldn't be shown if no repository connectors.

If a repository with sync_per_request is defined, I get warning Can't synchronize with repository .... However, it is unable to disable sync_per_request caused by invisible repository admin page and no repository connectors.

I consider we should show repository admin page if Trac environment has repositories and no repository connectors. Thoughts?

trac/versioncontrol/admin.py

@@ -179 +179 @@
     # IAdminPanelProvider methods
 
     def get_admin_panels(self, req):
-        types = RepositoryManager(self.env).get_supported_types()
-        if types and 'VERSIONCONTROL_ADMIN' \
-                      in req.perm('admin', 'versioncontrol/repository'):
+        rm = RepositoryManager(self.env)
+        if 'VERSIONCONTROL_ADMIN' in \
+                req.perm('admin', 'versioncontrol/repository') and \
+                (rm.get_supported_types() or rm.get_all_repositories()):
             yield ('versioncontrol', _('Version Control'), 'repository',
                    _('Repositories'))
 

[Ryan J Ollos]

Good find. I think we should definitely deal with this corner-case. What about skipping repository synchronization in RepositoryManager.pre_process_request?

trac/versioncontrol/api.py

@@ -356 +356 @@
 
     def pre_process_request(self, req, handler):
         from trac.web.chrome import Chrome, add_warning
-        if handler is not Chrome(self.env):
+        if handler is not Chrome(self.env) and self.get_supported_types():
             for repo_info in self.get_all_repositories().values():
                 if not as_bool(repo_info.get('sync_per_request')):
                     continue

[Jun Omae]

I think it would be better to skip sync if the repository type isn't supported for each repository with sync_per_request.

trac/versioncontrol/api.py

@@ -357 +357 @@
     def pre_process_request(self, req, handler):
         from trac.web.chrome import Chrome, add_warning
         if handler is not Chrome(self.env):
+            supported_types = set(self.get_supported_types())
             for repo_info in self.get_all_repositories().values():
                 if not as_bool(repo_info.get('sync_per_request')):
                     continue
+                rtype = repo_info.get('type') or self.default_repository_type
+                if rtype not in supported_types:
+                    continue
                 start = time_now()
                 repo_name = repo_info['name'] or '(default)'
                 try:

[Ryan J Ollos]

Checking supported type for each repository sounds good. I'd also be okay with equivalent functionality that uses exception handling.

trac/versioncontrol/api.py

@@ -40 +40 @@
     """Exception raised when a repository is invalid."""
 
 
+class InvalidConnector(TracError):
+    """Exception raised when a repository connector is invalid."""
+
+
 class IRepositoryConnector(Interface):
     """Provide support for a specific version control system."""
 
@@ -365 +369 @@
                 try:
                     repo = self.get_repository(repo_info['name'])
                     repo.sync()
+                except InvalidConnector:
+                    continue
                 except TracError as e:
                     add_warning(req,
                         _("Can't synchronize with repository \"%(name)s\" "
@@ -565 +571 @@
         :return: if no corresponding repository was defined,
                  simply return `None`.
 
+        :raises InvalidConnector: if the repository connector cannot be
+                                  opened.
         :raises InvalidRepository: if the repository cannot be opened.
         """
         reponame = reponame or ''
@@ -768 +776 @@
             if prio >= 0: # no error condition
                 return connector
             else:
-                raise TracError(
+                raise InvalidConnector(
                     _('Unsupported version control system "%(name)s"'
                       ': %(error)s', name=rtype,
                       error=to_unicode(connector.error)))
         else:
-            raise TracError(
+            raise InvalidConnector(
                 _('Unsupported version control system "%(name)s": '
                   'Can\'t find an appropriate component, maybe the '
                   'corresponding plugin was not enabled? ', name=rtype))

[Jun Omae]

That changes look good to me!

[Ryan J Ollos]

Thanks, I'll look at adding a test for the changes.

[Ryan J Ollos]

Fixed on 1.2-stable in r15123, merged to trunk in r15124.