launchpad-reviewers team mailing list archive

[William Grant <me@xxxxxxxxxxxxxxxxxx>]

Review: Approve code



Diff comments:

> 
> === modified file 'lib/lp/code/browser/configure.zcml'
> --- lib/lp/code/browser/configure.zcml	2016-10-14 15:44:39 +0000
> +++ lib/lp/code/browser/configure.zcml	2016-10-14 16:43:43 +0000
> @@ -881,6 +887,18 @@
>          permission="launchpad.AnyPerson"
>          name="+edit-subscription"
>          template="../templates/gitrepository-edit-subscription.pt"/>
> +    <browser:page
> +        for="lp.code.interfaces.gitrepository.IGitRepository"
> +        class="lp.code.browser.gitrepository.GitRepositoryRequestImportView"
> +        permission="launchpad.AnyPerson"
> +        name="+request-import"
> +        template="../templates/inline-form-only-buttons.pt"/>
> +    <browser:page
> +        for="lp.code.interfaces.gitrepository.IGitRepository"
> +        class="lp.code.browser.gitrepository.GitRepositoryTryImportAgainView"
> +        permission="launchpad.AnyPerson"
> +        name="+try-again"
> +        template="../templates/inline-form-only-buttons.pt"/>

Can you make these two AnyAllowedPerson? We need to update a whole lot of others to that, but we might as well not make the situation any worse.

>      <adapter
>          provides="lp.services.webapp.interfaces.IBreadcrumb"
>          for="lp.code.interfaces.gitrepository.IGitRepository"
> 
> === modified file 'lib/lp/code/browser/gitrepository.py'
> --- lib/lp/code/browser/gitrepository.py	2016-10-14 15:44:39 +0000
> +++ lib/lp/code/browser/gitrepository.py	2016-10-14 16:43:43 +0000
> @@ -667,6 +677,83 @@
>          return self, ()
>  
>  
> +class GitRepositoryRequestImportView(LaunchpadFormView):
> +    """The view to provide an 'Import now' button on the repository index page.
> +
> +    This only appears on the page of a repository with an associated code
> +    import that is being actively imported and where there is a import
> +    scheduled at some point in the future.
> +    """
> +
> +    schema = IGitRepository
> +    field_names = []
> +
> +    form_style = "display: inline"
> +
> +    @property
> +    def next_url(self):
> +        return canonical_url(self.context)
> +
> +    @action("Import Now", name="request")
> +    def request_import_action(self, action, data):
> +        try:
> +            self.context.code_import.requestImport(
> +                self.user, error_if_already_requested=True)
> +            self.request.response.addNotification(
> +                "Import will run as soon as possible.")
> +        except CodeImportNotInReviewedState:
> +            self.request.response.addNotification(
> +                "This import is no longer being updated automatically.")
> +        except CodeImportAlreadyRunning:
> +            self.request.response.addNotification(
> +                "The import is already running.")
> +        except CodeImportAlreadyRequested as e:
> +            user = e.requesting_user
> +            adapter = queryAdapter(user, IPathAdapter, 'fmt')
> +            self.request.response.addNotification(
> +                structured("The import has already been requested by %s." %
> +                           adapter.link(None)))
> +
> +    @property
> +    def prefix(self):
> +        return "request%s" % self.context.id
> +
> +    @property
> +    def action_url(self):
> +        return "%s/@@+request-import" % canonical_url(self.context)
> +
> +
> +class GitRepositoryTryImportAgainView(LaunchpadFormView):
> +    """The view to provide an 'Try again' button on the repository index page.
> +
> +    This only appears on the page of a repository with an associated code
> +    import that is marked as failing.
> +    """
> +
> +    schema = IGitRepository
> +    field_names = []
> +
> +    @property
> +    def next_url(self):
> +        return canonical_url(self.context)
> +
> +    @action("Try Again", name="tryagain")
> +    def request_try_again(self, action, data):
> +        if (self.context.code_import.review_status !=
> +            CodeImportReviewStatus.FAILING):
> +            self.request.response.addNotification(
> +                "The import is now %s."
> +                % self.context.code_import.review_status.name)
> +        else:
> +            self.context.code_import.tryFailingImportAgain(self.user)
> +            self.request.response.addNotification(
> +                "Import will be tried again as soon as possible.")
> +
> +    @property
> +    def prefix(self):
> +        return "tryagain"

These seem like direct copies of views from branch.py but with s/IBranch/IGitRepository/. The fact that they define a schema but then set field_names = [] also doesn't suggest to me that the schema actually matters. Can we just reuse the existing views, maybe moving them out to codeimport.py?

> +
> +
>  class GitRepositoryDeletionView(LaunchpadFormView):
>  
>      schema = IGitRepository


-- 
https://code.launchpad.net/~cjwatson/launchpad/codeimport-git-edit-views/+merge/308398
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.