launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Review: Approve



Diff comments:

> diff --git a/lib/lp/answers/browser/question.py b/lib/lp/answers/browser/question.py
> index 757da13..972993e 100644
> --- a/lib/lp/answers/browser/question.py
> +++ b/lib/lp/answers/browser/question.py
> @@ -269,6 +270,16 @@ class QuestionNavigation(Navigation):
>          return self.context.messages[index]
>  
>  
> +class QuestionMessageNavigation(Navigation):
> +    """Navigation for the IQuestionMessage."""
> +
> +    usedfor = IQuestionMessage
> +
> +    @stepthrough('revisions')
> +    def traverse_revisions(self, revision):
> +        return self.context.getRevisionByNumber(int(revision))

This should return None if it fails to interpret the revision as an integer.

> +
> +
>  class QuestionBreadcrumb(Breadcrumb):
>      """Builds a breadcrumb for an `IQuestion`."""
>  
> diff --git a/lib/lp/bugs/browser/bugcomment.py b/lib/lp/bugs/browser/bugcomment.py
> index e40967f..a147ded 100644
> --- a/lib/lp/bugs/browser/bugcomment.py
> +++ b/lib/lp/bugs/browser/bugcomment.py
> @@ -57,6 +59,15 @@ from lp.services.webapp.interfaces import ILaunchBag
>  COMMENT_ACTIVITY_GROUPING_WINDOW = timedelta(minutes=5)
>  
>  
> +class BugCommentNavigation(Navigation):
> +    """Navigation for the `IBugComment`."""
> +    usedfor = IBugComment
> +
> +    @stepthrough('revisions')
> +    def traverse_revisions(self, revision):
> +        return self.context.getRevisionByNumber(int(revision))

This should return None if it fails to interpret the revision as an integer.

> +
> +
>  def build_comments_from_chunks(
>          bugtask, truncate=False, slice_info=None, show_spam_controls=False,
>          user=None, hide_first=False):
> diff --git a/lib/lp/code/browser/codereviewcomment.py b/lib/lp/code/browser/codereviewcomment.py
> index 0d0496d..1892eb8 100644
> --- a/lib/lp/code/browser/codereviewcomment.py
> +++ b/lib/lp/code/browser/codereviewcomment.py
> @@ -56,10 +56,21 @@ from lp.services.webapp import (
>      ContextMenu,
>      LaunchpadView,
>      Link,
> +    Navigation,
> +    stepthrough,
>      )
>  from lp.services.webapp.interfaces import ILaunchBag
>  
>  
> +class CodeReviewCommentNavigation(Navigation):
> +    """Navigation for the `ICodeReviewComment`."""
> +    usedfor = ICodeReviewComment
> +
> +    @stepthrough('revisions')
> +    def traverse_revisions(self, revision):
> +        return self.context.getRevisionByNumber(int(revision))

This should return None if it fails to interpret the revision as an integer.

> +
> +
>  class ICodeReviewDisplayComment(IComment, ICodeReviewComment):
>      """Marker interface for displaying code review comments."""
>      message = Object(schema=IMessage, title=_('The message.'))
> diff --git a/lib/lp/services/messages/interfaces/message.py b/lib/lp/services/messages/interfaces/message.py
> index 73edea1..d05dc44 100644
> --- a/lib/lp/services/messages/interfaces/message.py
> +++ b/lib/lp/services/messages/interfaces/message.py
> @@ -88,7 +88,14 @@ class IMessageCommon(Interface):
>          Reference(title=_('Person'), schema=Interface,
>                    required=False, readonly=True))
>  
> -    revisions = Attribute(_('Message revision history'))
> +    revisions = exported(CollectionField(
> +        title=_("Message revision history"),
> +        description=_(
> +            "Revision history of this message, sorted in descending order."),

The implementation sorts in ascending order (following https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/402211).

> +        # Really IMessageRevision, patched in _schema_circular_imports.
> +        value_type=Reference(schema=Interface),
> +        required=False, readonly=True), as_of="devel")
> +
>      datecreated = exported(
>          Datetime(title=_('Date Created'), required=True, readonly=True),
>          exported_as='date_created')
> diff --git a/lib/lp/services/messages/interfaces/messagerevision.py b/lib/lp/services/messages/interfaces/messagerevision.py
> index 133924f..2781521 100644
> --- a/lib/lp/services/messages/interfaces/messagerevision.py
> +++ b/lib/lp/services/messages/interfaces/messagerevision.py
> @@ -31,21 +37,26 @@ class IMessageRevisionView(Interface):
>  
>      revision = Int(title=_("Revision number"), required=True, readonly=True)
>  
> -    content = Text(
> +    content = exported(Text(
>          title=_("The message at the given revision"),
> -        required=False, readonly=True)
> +        required=False, readonly=True))
>  
>      message = Reference(
>          title=_('The current message of this revision.'),
>          schema=IMessage, required=True, readonly=True)
>  
> -    date_created = Datetime(
> +    message_implementation = Reference(
> +        title=_('The message implementation (BugComment, QuestionMessage or '
> +                'CodeReviewComment) related to this revision'),
> +        schema=Interface, required=True, readonly=True)

We should probably have a marker interface covering all these at some point, rather than having to just use `Interface`.

> +
> +    date_created = exported(Datetime(
>          title=_("The time when this message revision was created."),
> -        required=True, readonly=True)
> +        required=True, readonly=True))
>  
> -    date_deleted = Datetime(
> +    date_deleted = exported(Datetime(
>          title=_("The time when this message revision was created."),
> -        required=False, readonly=True)
> +        required=False, readonly=True))
>  
>      chunks = Attribute(_('Message pieces'))
>  
> diff --git a/lib/lp/services/messages/model/messagerevision.py b/lib/lp/services/messages/model/messagerevision.py
> index f4a1466..535ff66 100644
> --- a/lib/lp/services/messages/model/messagerevision.py
> +++ b/lib/lp/services/messages/model/messagerevision.py
> @@ -57,6 +57,27 @@ class MessageRevision(StormBase):
>          self.date_created = date_created
>          self.date_deleted = date_deleted
>  
> +    @property
> +    def message_implementation(self):
> +        from lp.bugs.model.bugmessage import BugMessage
> +        from lp.code.model.codereviewcomment import CodeReviewComment
> +        from lp.answers.model.questionmessage import QuestionMessage
> +
> +        store = IStore(self)
> +        (identifier, ) = store.execute("""
> +            SELECT 'bug' FROM BugMessage WHERE message = %s
> +            UNION
> +            SELECT 'question' FROM QuestionMessage WHERE message = %s
> +            UNION
> +            SELECT 'mp' FROM CodeReviewMessage WHERE message = %s;
> +        """, params=[self.message_id] * 3).get_one()

Nice trick.

> +        id_to_class = {
> +            "bug": BugMessage,
> +            "question": QuestionMessage,
> +            "mp": CodeReviewComment}
> +        klass = id_to_class[identifier]
> +        return store.find(klass, klass.message == self.message_id).one()
> +
>      @cachedproperty
>      def chunks(self):
>          return list(IStore(self).find(


-- 
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/402285
Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-api.