On 2013-11-07 17:39, Joachim Dreimann wrote: 

> On 6 November 2013
18:15, Gary Martin <[email protected]> wrote:
> 
>> Hi Olivier,
Thanks for bringing all that up! On 06/11/13 15:06, Olivier Mauras
wrote: 
>> 
>>> Hello, I met Gary Martin this morning on #trac and
discussing quickly about the look & feel of Bloodhound i ended up
installing it on a test server. Here's a compilation of the points that
i find "lacking": 1/ Blocking points for a production adoption - indeed
that's only my opinion * Product owner should automatically have admin
rights to this product. I plan on using Bloodhound in a multi product /
multi users setup. I can't imagine it working without delegating the
product admin rights to their owners
>> I am pretty happy with named
owners of products, or more generally resources, being given rights
associated with admin of the resource. It might be useful if the
permissions admin settings for a product reflected that in some sense.

>> 
>>> * Products should be able to have their own source repo If
rights are delegated, I don't see any valid reason for owners to not be
able to add their own repos
>> I think that there may be some good
use-cases for this, particularly when combined with the rights to access
the product's admin pages. For instance it could be that a repository
for a product should never even be known about in another product. The
current solution forcing repository definitions globally and allowing
local links means that the admin pages would have to be restricted to
those that are effectively allowed access to all. It could instead be
delegated. In the end, there is still the current requirement that
repositories have to be on disk and a trac-admin command has to be run
by someone with appropriate access to the server but that doesn't fully
justify the restrictions.
> 
> Tabs should remember the place you were
last within the tab context, so
> that you don't lose context every time
to switch tabs. Currently tabs
> always return you back to the landing
page for that tab, which is a poor
> experience in my opinion.
> 
>
Thanks for your feedback!
> 
> Joe
> 
>>> * Not knowing where you are
This is indeed linked to the previous point, wiki and source pages don't
show the product in breadcrumb
>> I would definitely argue for extending
the breadcrumb to go across all pages for this purpose. 
>> 
>>> 3/ Eye
candy * Source browser CSS may not be the easiest to read All lines to
the same color make it difficult to catch things Age color highlight is
too light and/or too close in colors * There's place on the mainnav tabs
As there's place available and that one can enable timeline and roadmap
from base.ini why not add them to mainnav() to not have then in "More"
tab?
>> The timeline has the virtue that it crosses all aspects of the
system. There is probably a case for having a link, particularly if
there are pages that do not have activity areas.

Please find attached
two patches.
First one grants product owner admin rights to his
products, and change the repository management part to the one used by
global one. This makes the product owner to create/delete/alias
repositories. 
For the moment it's global, it would require some changes
at DB level to make the repositories isolated per product. I think a new
table - "product_repositories" - with a new "DbRepositoryProvider" class
would be the less intrusive...

The second small patch modifies the
theme to get "Source" tab to point to product/<id>/browser instead of
getting the wiki.
This indeed gives a "No node /" error as i haven't yet
found my way in the code that would point the product browser to the
repository.

Sorry for the nasty two patches, i worked on installed app
instead of the code... This is all based on the latest stable
0.7.0.

Hope this will help.

Olivier 
 
diff --git a/product_admin.py b/product_admin.py
index 9c0fc42..df69e04 100644
--- a/product_admin.py
+++ b/product_admin.py
@@ -18,6 +18,8 @@
 
 """Admin panels for product management"""
 
+from genshi.builder import tag
+
 from trac.admin.api import IAdminCommandProvider, AdminCommandError,\
     AdminCommandManager
 from trac.admin.console import TracAdmin, TRAC_VERSION
@@ -27,9 +29,9 @@ from trac.config import *
 from trac.perm import PermissionSystem
 from trac.resource import ResourceNotFound
 from trac.ticket.admin import TicketAdminPanel, _save_config
-from trac.util import lazy
-from trac.util.text import print_table, to_unicode, printerr, printout
-from trac.util.translation import _, N_, gettext, ngettext
+from trac.util import lazy, as_bool
+from trac.util.text import print_table, to_unicode, printerr, printout, breakable_path, normalize_whitespace
+from trac.util.translation import _, N_, gettext, ngettext, tag_
 from trac.web.api import HTTPNotFound, IRequestFilter, IRequestHandler
 from trac.web.chrome import Chrome, add_notice, add_warning
 
@@ -37,9 +39,10 @@ from multiproduct.env import ProductEnvironment
 from multiproduct.model import Product
 from multiproduct.perm import sudo
 
-import multiproduct.versioncontrol
+#import multiproduct.versioncontrol
 import trac.versioncontrol.admin
-from trac.versioncontrol import DbRepositoryProvider, RepositoryManager
+from trac.versioncontrol import DbRepositoryProvider, RepositoryManager, \
+                                is_default
 from multiproduct.util import ReplacementComponent
 
 #--------------------------
@@ -335,8 +338,7 @@ class ProductAdminModule(Component):
         """
         if isinstance(self.env, ProductEnvironment) and \
                 handler is AdminModule(self.env) and \
-                not req.perm.has_permission('TRAC_ADMIN') and \
-                req.perm.has_permission('PRODUCT_ADMIN'):
+                not req.perm.has_permission('PRODUCT_ADMIN'):
             # Intercept admin request
             return self
         return handler
@@ -410,9 +412,27 @@ class ProductRepositoryAdminPanel(ReplacementComponent, trac.versioncontrol.admi
 
     def get_admin_panels(self, req):
         if 'VERSIONCONTROL_ADMIN' in req.perm:
-            yield ('versioncontrol', _('Version Control'), 'repository',
-                   _('Repository Links') if isinstance(self.env, ProductEnvironment)
-                    else _('Repositories'))
+            yield ('versioncontrol', _('Version Control'), 'repository', 
+                _('Repositories'))
+
+    def _extend_info(self, reponame, info, editable):
+        """Extend repository info for rendering."""
+        info['name'] = reponame                                                                                                                                                                                                                                                  
+        if info.get('dir') is not None:
+            info['prettydir'] = breakable_path(info['dir']) or ''
+        info['hidden'] = as_bool(info.get('hidden'))
+        #info['editable'] = editable
+        info['editable'] = True
+        if not info.get('alias'):
+            try:
+                self.log.debug(reponame)
+                repos = RepositoryManager(self.env.parent).get_repository(reponame)
+                youngest_rev = repos.get_youngest_rev()
+                info['rev'] = youngest_rev
+                info['display_rev'] = repos.display_rev(youngest_rev)
+            except Exception:
+                pass
+        return info
 
     def render_admin_panel(self, req, category, page, path_info):
         if not isinstance(self.env, ProductEnvironment):
@@ -420,41 +440,143 @@ class ProductRepositoryAdminPanel(ReplacementComponent, trac.versioncontrol.admi
                 req, category, page, path_info)
 
         req.perm.require('VERSIONCONTROL_ADMIN')
-        db_provider = self.env[DbRepositoryProvider]
-
-        if req.method == 'POST' and db_provider:
-            if req.args.get('remove'):
-                repolist = req.args.get('sel')
-                if repolist:
-                    if isinstance(repolist, basestring):
-                        repolist = [repolist, ]
-                    for reponame in repolist:
-                        db_provider.unlink_product(reponame)
-            elif req.args.get('addlink') is not None and db_provider:
-                reponame = req.args.get('repository')
-                db_provider.link_product(reponame)
-            req.redirect(req.href.admin(category, page))
-
-        # Retrieve info for all product repositories
-        rm_product = RepositoryManager(self.env)
-        rm_product.reload_repositories()
-        all_product_repos = rm_product.get_all_repositories()
-        repositories = dict((reponame, self._extend_info(
-                                reponame, info.copy(), True))
-                            for (reponame, info) in
-                                all_product_repos.iteritems())
-        types = sorted([''] + rm_product.get_supported_types())
-
-        # construct a list of all repositores not linked to this product
         rm = RepositoryManager(self.env.parent)
         all_repos = rm.get_all_repositories()
-        unlinked_repositories = dict([(k, all_repos[k]) for k in
-            sorted(set(all_repos) - set(all_product_repos))])
+        db_provider = self.env[DbRepositoryProvider]
+
+        if path_info:
+            # Detail view
+            self.log.debug(path_info)
+            reponame = path_info if not is_default(path_info) else ''
+            info = all_repos.get(reponame)
+            if info is None:
+                raise TracError(_("Repository '%(repo)s' not found",
+                                  repo=path_info))
+            if req.method == 'POST':
+                if req.args.get('cancel'):
+                    req.redirect(req.href.admin(category, page))
+
+                elif db_provider and req.args.get('save'):
+                    # Modify repository
+                    changes = {}
+                    for field in db_provider.repository_attrs:
+                        value = normalize_whitespace(req.args.get(field))
+                        if (value is not None or field == 'hidden') \
+                                and value != info.get(field):
+                            changes[field] = value
+                    if 'dir' in changes \
+                            and not self._check_dir(req, changes['dir']):
+                        changes = {}
+                    if changes:
+                        db_provider.modify_repository(reponame, changes)
+                        add_notice(req, _('Your changes have been saved.'))
+                    name = req.args.get('name')
+                    resync = tag.tt('trac-admin $ENV repository resync "%s"'
+                                    % (name or '(default)'))
+                    if 'dir' in changes:
+                        msg = tag_('You should now run %(resync)s to '
+                                   'synchronize Trac with the repository.',
+                                   resync=resync)
+                        add_notice(req, msg)
+                    elif 'type' in changes:
+                        msg = tag_('You may have to run %(resync)s to '
+                                   'synchronize Trac with the repository.',
+                                   resync=resync)
+                        add_notice(req, msg)
+                    if name and name != path_info and not 'alias' in info:
+                        cset_added = tag.tt('trac-admin $ENV changeset '
+                                            'added "%s" $REV'
+                                            % (name or '(default)'))
+                        msg = tag_('You will need to update your post-commit '
+                                   'hook to call %(cset_added)s with the new '
+                                   'repository name.', cset_added=cset_added)
+                        add_notice(req, msg)
+                    if changes:
+                        req.redirect(req.href.admin(category, page))
+
+            Chrome(self.env).add_wiki_toolbars(req)
+            data = {'view': 'detail', 'reponame': reponame}
 
-        data = {'types': types, 'default_type': rm_product.repository_type,
-                'repositories': repositories,
-                'unlinked_repositories': unlinked_repositories}
-        return 'repository_links.html', data
+        else:
+            if req.method == 'POST':
+                # Add a repository
+                if db_provider and req.args.get('add_repos'):
+                    name = req.args.get('name')
+                    type_ = req.args.get('type')
+                    # Avoid errors when copy/pasting paths
+                    dir = normalize_whitespace(req.args.get('dir', ''))
+                    if name is None or type_ is None or not dir:
+                        add_warning(req, _('Missing arguments to add a '
+                                            'repository.'))
+                    elif self._check_dir(req, dir):
+                        db_provider.add_repository(name, dir, type_)
+                        name = name or '(default)'
+                        add_notice(req, _('The repository "%(name)s" has been '
+                                          'added.', name=name))
+                        resync = tag.tt('trac-admin $ENV repository resync '
+                                        '"%s"' % name)
+                        msg = tag_('You should now run %(resync)s to '
+                                   'synchronize Trac with the repository.',
+                                    resync=resync)
+                        add_notice(req, msg)
+                        cset_added = tag.tt('trac-admin $ENV changeset '
+                                            'added "%s" $REV' % name)
+                        msg = tag_('You should also set up a post-commit hook '
+                                   'on the repository to call %(cset_added)s '
+                                   'for each committed changeset.',
+                                   cset_added=cset_added)
+                        add_notice(req, msg)
+                        req.redirect(req.href.admin(category, page))
+
+                # Add a repository alias
+                elif db_provider and req.args.get('add_alias'):
+                    name = req.args.get('name')
+                    alias = req.args.get('alias')
+                    if name is not None and alias is not None:
+                        db_provider.add_alias(name, alias)
+                        add_notice(req, _('The alias "%(name)s" has been '
+                                          'added.', name=name or '(default)'))
+                        req.redirect(req.href.admin(category, page))
+                    add_warning(req, _('Missing arguments to add an '
+                                       'alias.'))
+
+                # Refresh the list of repositories
+                elif req.args.get('refresh'):
+                    req.redirect(req.href.admin(category, page))
+
+                # Remove repositories
+                elif db_provider and req.args.get('remove'):
+                    sel = req.args.getlist('sel')
+                    if sel:
+                        for name in sel:
+                            db_provider.remove_repository(name)
+                        add_notice(req, _('The selected repositories have '
+                                          'been removed.'))
+                        req.redirect(req.href.admin(category, page))
+                    add_warning(req, _('No repositories were selected.'))
+
+            data = {'view': 'list'}
+
+        # Force repo refresh - should already happen :/
+        rm.reload_repositories()
+
+        #self.log.debug(all_repos)
+
+        db_repos = {}
+        if db_provider is not None:
+            db_repos = dict(db_provider.get_repositories())
+
+        # Prepare common rendering data
+        repositories = dict((reponame, self._extend_info(reponame, info.copy(),
+                                                         reponame in db_repos))
+                            for (reponame, info) in all_repos.iteritems())
+
+        #self.log.debug(repositories)
+        types = sorted([''] + rm.get_supported_types())
+        data.update({'types': types, 'default_type': rm.repository_type,
+                     'repositories': repositories})
+
+        return 'admin_repositories.html', data
 
 trac.versioncontrol.admin.RepositoryAdminPanel = ProductRepositoryAdminPanel
 

diff --git a/theme.py b/theme.py
index 6c7f81b..8686a26 100644
--- a/theme.py
+++ b/theme.py
@@ -443,8 +443,7 @@ class BloodhoundTheme(ThemeBase):
             bm = self.env[BrowserModule]
             if bm and not list(bm.get_navigation_items(req)):
                 yield ('mainnav', 'browser', 
-                       tag.a(_('Browse Source'),
-                             href=req.href.wiki('TracRepositoryAdmin')))
+                       tag.a(_('Browse Source'), href=req.href.browser()))
 
 
 class QuickCreateTicketDialog(Component):

Reply via email to