On Tue, Nov 12, 2013 at 11:58 AM, Gary Martin <gary.martin@wandisco.com>wrote:
> Hi,
>
:)
>
> Just wondering how people felt about Olivier's contribution so far and the
> idea in general. Obviously the code does need a bit more work as we
> probably need to have product repositories isolated from the global
> environment.
>
IMO what we need is UI to hide the underlying complexity of product-repos
links and give product admins the chance to create / fork / import their
own repos thus giving them the illusion of creating new repos in a given
product context .
>
> While I think we definitely should maintain global repositories that
> products can link to,
+
> the ability to keep a repository private to a specific product, even in
> the admin, makes a lot of sense to me.
>
+
... now ... below I add my comment about both patches
>
> Cheers,
> Gary
>
>
> On 07/11/13 16:40, Olivier Mauras wrote:
>
>>
>> Please find attached two patches.
>> First one grants product owner admin rights to his products,
>>
>
Like I just said this should be the case in current implementation via
permission policies .
> and change the repository management part to the one used by global one.
>> This makes the product owner to create/delete/alias repositories.
>>
>
I really do not want this to happen because this implies that product
admins will have knowledge of file-system path and access to perform some
actions beyond web UI , which I do not recommend in many kinds of
deployments , especially those similar to forges where most products are
unrelated and many people actually manage their own products .
@olivier : firstly ... have you run the test suite after with your patches
applied ? What are the results ?
I have other objections to the first patch that I'll mention below .
[...]
> The second small patch modifies the theme to get "Source" tab to point
>> to product/<id>/browser instead of getting the wiki.
>>
>
JFTR , even if your patch is ok , this is a design decision so needs to be
discussed a bit further .
> 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
>>
>>
>> bh_p1.patch
>>
>>
>> diff --git a/product_admin.py b/product_admin.py
>> index 9c0fc42..df69e04 100644
>> --- a/product_admin.py
>> +++ b/product_admin.py
>>
> [...]
> @@ -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
>>
>
I'm strongly against this change ... This is aimed at granting
PRODUCT_ADMIN users with access to admin panels even if they lack
TRAC_ADMIN permission . What's the goal of your modifications ?
> @@ -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):
>>
>
Does this belong in Bloodhound theme template override functions i.e.
bhtheme.theme ?
> @@ -420,41 +440,143 @@ class
>> ProductRepositoryAdminPanel(ReplacementComponent, trac.versioncontrol.admi
>>
>
> [...]
I'm not quite sure ... but maybe I did not understand all the details
involved .
[...]
--
Regards,
Olemis - @olemislc
|