[Launchpad-reviewers] [Merge] ~pappacena/launchpad:delete-message-revision-ui into launchpad:master
Thiago F. Pappacena has proposed merging
~pappacena/launchpad:delete-message-revision-ui into launchpad:master.
Commit message:
Allowing users to delete message revision history
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403471
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~pappacena/launchpad:delete-message-revision-ui into launchpad:master.
diff --git a/lib/canonical/launchpad/icing/css/base.scss b/lib/canonical/launchpad/icing/css/base.scss
index 1e33ca7..cff195c 100644
--- a/lib/canonical/launchpad/icing/css/base.scss
+++ b/lib/canonical/launchpad/icing/css/base.scss
@@ -624,6 +624,13 @@ body {
display: none;
padding-left: 20px;
padding-bottom: 10px;
+
+// If the content was deleted, we show a default message with
+// this CSS class.
+.deleted-content {
+padding: 5px;
+opacity: 50%;
+}
}
}
diff --git a/lib/lp/answers/templates/questionmessage-display.pt b/lib/lp/answers/templates/questionmessage-display.pt
index c1ed29a..346f11b 100644
--- a/lib/lp/answers/templates/questionmessage-display.pt
+++ b/lib/lp/answers/templates/questionmessage-display.pt
@@ -8,7 +8,8 @@
tal:define="css_classes view/getBoardCommentCSSClass"
tal:attributes="class string:${css_classes};
id string:comment-${context/index};
- data-baseurl context/fmt:url">
+ data-baseurl context/fmt:url;
+ data-i-can-edit view/can_edit">
@@ -23,6 +24,9 @@
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:delete-message-revision-ui into launchpad:master
We are missing some tests (and applying the tag to bug comments and code review), but this works fine the way it is on questions. -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403471 Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:delete-message-revision-ui into launchpad:master. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:git-repo-async-privacy-virtualrefs into launchpad:master
The proposal to merge ~pappacena/launchpad:git-repo-async-privacy-virtualrefs into launchpad:master has been updated. Commit message changed to: Doing virtual refs synchronization when running git repository privacy change background task For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/393075 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:git-repo-async-privacy-virtualrefs into launchpad:master. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-ui-list-revisions-codereview into launchpad:master
The proposal to merge ~pappacena/launchpad:comment-editing-ui-list-revisions-codereview into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403285 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-ui-list-revisions-bugcomment. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-ui-list-revisions-bugcomment into launchpad:master
The proposal to merge ~pappacena/launchpad:comment-editing-ui-list-revisions-bugcomment into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403279 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-ui-list-revisions. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-ui-list-revisions into launchpad:master
The proposal to merge ~pappacena/launchpad:comment-editing-ui-list-revisions into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403213 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-ui-list-revisions. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-ui-list-revisions into launchpad:master
Pushed the requested changes. This should be good to go now.
Diff comments:
> diff --git a/lib/lp/answers/browser/question.py
> b/lib/lp/answers/browser/question.py
> index 9d4986d..52cff40 100644
> --- a/lib/lp/answers/browser/question.py
> +++ b/lib/lp/answers/browser/question.py
> @@ -1195,13 +1195,11 @@ class QuestionMessageDisplayView(LaunchpadView):
> return check_permission('launchpad.Moderate', self.context)
>
> def getBoardCommentCSSClass(self):
> -css_classes = ["boardComment"]
> +css_classes = ["boardComment", "editable-message"]
> if not self.context.visible:
> # If a comment that isn't visible is being rendered, it's being
> # rendered for an admin or registry_expert.
> css_classes.append("adminHiddenComment")
> -if self.can_edit:
> -css_classes.append("editable-message")
The displaying of the pencil icon is already guarded by
`tal:condition="view/can_edit"`.
We need to add `editable-message` class on every message so we can add the link
to the revision history list. Everyone is able to see such list, regardless on
having or not permission to edit the message itself.
> return " ".join(css_classes)
>
> @property
> diff --git a/lib/lp/services/messages/javascript/messages.edit.js
> b/lib/lp/services/messages/javascript/messages.edit.js
> index 3c4c0e5..912a24c 100644
> --- a/lib/lp/services/messages/javascript/messages.edit.js
> +++ b/lib/lp/services/messages/javascript/messages.edit.js
> @@ -168,6 +179,74 @@ YUI.add('lp.services.messages.edit', function(Y) {
> elements.update_btn.getDOMNode().disabled = false;
> };
>
> +module.fillMessageRevisions = function(elements, revisions) {
> +// Position the message revision list element.
> +revisions = revisions.reverse();
> +var revisions_container = elements.container.one(
> +".message-revision-container");
> +var last_edit_el = elements.last_edit.getDOMNode();
> +var target_position = last_edit_el.getBoundingClientRect();
> +var nodes_holder = revisions_container.one(".message-revision-list");
> +var template = revisions_container.one(
> +"script[type='text/template']").getDOMNode().innerHTML;
> +
> +revisions_container.setStyle('left', target_position.left);
> +revisions_container.setStyle('display', 'block');
> +revisions_container.one(".message-revision-close").on(
> +"click", function() {
> +nodes_holder.getDOMNode().innerHTML = '';
> +revisions_container.setStyle('display', 'none');
> +});
> +
> +var content = "";
> +revisions.forEach(function(rev) {
> +var attrs = rev.getAttrs();
> +var date_created = new Date(attrs.date_created);
> +attrs.date_created = (
> +date_created.toISOString().substr(0,10) + ", " +
> +date_created.toLocaleTimeString());
It makes sense. I'll use toLocaleString here.
I thought also about implementing something in JS to display it as a relative
date (like 'x days ago', 'y months ago', etc), but I guess this can go in
another MP.
> +attrs.content = module.htmlify_msg(attrs.content);
> +content += Y.Lang.sub(template, attrs);
> +});
> +
> +nodes_holder.getDOMNode().innerHTML = content;
> +nodes_holder.all(".message-revision-item").each(function(rev_item) {
> +rev_item.one(".message-revision-title").on('click', function() {
> +nodes_holder.all('.message-revision-body').setStyle(
> +'display', 'none');
> +var body = rev_item.one('.message-revision-body');
> +var current_display = body.getStyle('display');
> +body.setStyle(
> +'display', current_display === 'block'? 'none' :
> 'block');
> +nodes_holder.all(".message-revision-item").removeClass(
> +'active');
> +rev_item.addClass('active');
> +});
> +});
> +};
> +
> +module.onLastEditClick = function(elements, baseurl) {
> +// Hide all open revision containers.
> +Y.all('.message-revision-container').each(function(container) {
> +container.setStyle('display', 'none');
> +});
> +
> +var url = "/api/devel" + baseurl + "/revisions";
> +var config = {
> +on: {
> + success: function(response) {
> +module.fillMessageRevisions(elements, response.entries);
> + },
> + failure: function(err) {
> +alert("Error fetching revisions.");
> + }
> + },
> + // XXX pappacena 2021-05-19: Pagination will be needed here.
> + size: 100
> +};
> +
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-ui-list-revisions-codereview into launchpad:master
Thiago F. Pappacena has proposed merging
~pappacena/launchpad:comment-editing-ui-list-revisions-codereview into
launchpad:master with
~pappacena/launchpad:comment-editing-ui-list-revisions-bugcomment as a
prerequisite.
Commit message:
Adding comment revision lits to code review page
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403285
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~pappacena/launchpad:comment-editing-ui-list-revisions-codereview into
launchpad:master.
diff --git a/lib/lp/code/stories/branches/xx-code-review-comments.txt b/lib/lp/code/stories/branches/xx-code-review-comments.txt
index 30d3a26..d0575b0 100644
--- a/lib/lp/code/stories/branches/xx-code-review-comments.txt
+++ b/lib/lp/code/stories/branches/xx-code-review-comments.txt
@@ -38,6 +38,7 @@ to the main merge proposal page.
... print(extract_text(tag))
>>> print_comments('boardCommentDetails')
+Revision history for this message
Eric the Viking (eric) wrote ...
>>> print_comments('comment-text')
This is a very long comment about what things should be done to the
@@ -111,6 +112,7 @@ are also displayed in the new proposal.
source branch to land it. When this comment is replied to, it should
wrap the line properly.
>>> print_comments('boardCommentDetails', anon_browser, index=0)
+Revision history for this message
Eric the Viking (eric) wrote ... ago:
Posted in a previous version of this proposal #
>>> details = find_tags_by_class(
@@ -132,6 +134,7 @@ comment' link.
>>> eric_browser.getControl('Save Comment').click()
>>> print_comments('boardCommentDetails', eric_browser, index=2)
+Revision history for this message
Eric the Viking ... ago: #
>>> print_comments('boardCommentActivity', eric_browser, index=0)
review: Abstain (timeless)
diff --git a/lib/lp/code/templates/codereviewcomment-header.pt b/lib/lp/code/templates/codereviewcomment-header.pt
index 77c4650..3d2b976 100644
--- a/lib/lp/code/templates/codereviewcomment-header.pt
+++ b/lib/lp/code/templates/codereviewcomment-header.pt
@@ -3,6 +3,25 @@
xmlns:metal="http://xml.zope.org/namespaces/metal;
omit-tag="">
+
+
+Revision history for this message
+
+
+
+<div class='message-revision-item'>
+ <div class='message-revision-title'>
+ <a class="js-action">
+ Revision #{revision}, created at {date_created}
+ </a>
+ </div>
+ <div class='message-revision-body'>{content}</div>
+</div>
+
+
+
+
@@ -19,11 +38,12 @@
tal:content="context/comment_date/fmt:displaydate">
7 minutes ago
-(last edit (last edit ):
+tal:content="context/date_last_edited/fmt:displaydate" />):
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-ui-list-revisions-bugcomment into launchpad:master
Thiago F. Pappacena has proposed merging
~pappacena/launchpad:comment-editing-ui-list-revisions-bugcomment into
launchpad:master with ~pappacena/launchpad:comment-editing-ui-list-revisions as
a prerequisite.
Commit message:
Adding message revision list to bug comment
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403279
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~pappacena/launchpad:comment-editing-ui-list-revisions-bugcomment into
launchpad:master.
diff --git a/lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.txt b/lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.txt
index 2bf6a34..9016ea8 100644
--- a/lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.txt
+++ b/lib/lp/bugs/stories/bugattachments/xx-attachments-to-bug-report.txt
@@ -12,6 +12,7 @@ of bug #11 is:
>>> browser.open("http://bugs.launchpad.test/redfish/+bug/11;)
>>> comment_1 = find_tags_by_class(browser.contents, 'boardComment')[0]
>>> print(extract_text(comment_1))
+Revision history for this message
Valentina Commissari (tsukimi)
wrote
on 2007-03-15:
@@ -43,6 +44,7 @@ displayed as the bug report).
>>> browser.open("http://bugs.launchpad.test/redfish/+bug/11;)
>>> comment_0 = find_tags_by_class(browser.contents, 'boardComment')[0]
>>> print(extract_text(comment_0))
+Revision history for this message
Daniel Silverstone (kinnison)
wrote
on 2007-03-15:
diff --git a/lib/lp/bugs/stories/bugs/xx-bug-activity.txt b/lib/lp/bugs/stories/bugs/xx-bug-activity.txt
index 95d0d4f..1d75aa4 100644
--- a/lib/lp/bugs/stories/bugs/xx-bug-activity.txt
+++ b/lib/lp/bugs/stories/bugs/xx-bug-activity.txt
@@ -63,6 +63,7 @@ page.
... "Here's a comment for testing, like.")
>>> user_browser.getControl('Post Comment').click()
>>> print_comments(user_browser.contents, slice(None))
+Revision history for this message
In...
Bug Watch Updater (bug-watch-updater)
on 2007-12-18
@@ -70,6 +71,7 @@ page.
status:
Unknown → New
+Revision history for this message
No Privileges Person (no-priv)
...
#7
@@ -88,6 +90,7 @@ that have been added.
>>> user_browser.open('http://launchpad.test/bugs/15')
>>> print_comments(user_browser.contents, slice(None))
+Revision history for this message
In...
Bug
...
@@ -226,6 +229,7 @@ bundled with that comment in the UI.
Note that "Lookit, a change!" appears twice: once displaying the message
itself, and once again inside the textarea to edit the message.
>>> print_comments(admin_browser.contents)
+Revision history for this message
Foo Bar (name16)
wrote
... ago:
@@ -252,6 +256,7 @@ If a target of a bug task is changed the old and new value will be shown.
... ).value = 'linux-source-2.6.15'
>>> admin_browser.getControl("Save Changes").click()
>>> print_comments(admin_browser.contents)
+Revision history for this message
Foo Bar (name16)
wrote
... ago:
diff --git a/lib/lp/bugs/stories/bugs/xx-bug-create-question.txt b/lib/lp/bugs/stories/bugs/xx-bug-create-question.txt
index ed2fd5d..b094569 100644
--- a/lib/lp/bugs/stories/bugs/xx-bug-create-question.txt
+++ b/lib/lp/bugs/stories/bugs/xx-bug-create-question.txt
@@ -271,6 +271,7 @@ They read their comment that was appended to the bug report's messages.
>>> print(extract_text(
... find_tags_by_class(str(content), 'boardComment')[-1]))
+Revision history for this message
No Privileges Person (no-priv)
wrote
...
diff --git a/lib/lp/bugs/stories/bugs/xx-remote-bug-comments.txt b/lib/lp/bugs/stories/bugs/xx-remote-bug-comments.txt
index 7fa1e82..59e338d 100644
--- a/lib/lp/bugs/stories/bugs/xx-remote-bug-comments.txt
+++ b/lib/lp/bugs/stories/bugs/xx-remote-bug-comments.txt
@@ -11,6 +11,7 @@ entered within Launchpad itself.
>>> details = remote_bug_comment.find(
... attrs={'class': 'boardCommentDetails'})
>>> print(extract_text(details))
+Revision history for this message
In
Debian Bug tracker #308994,
josh (jbuhl-nospam)
@@ -21,7 +22,7 @@ entered within Launchpad itself.
Remote comments are decorated with the bug watch icon, to distinguish
them from comments posted directly by Launchpad users.
->>> print(details.find('img')['src'])
+>>> print(details.find_all('img')[1]['src'])
/@@/bug-remote
Since it's possible to reply to imported comments and have them
@@ -52,6 +53,7 @@ The new comment appears, formatted as a remote bug comment.
>>> new_bug_comment = find_tags_by
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:avoid-msg-edit-crash-on-deleted-messages into launchpad:master
The proposal to merge ~pappacena/launchpad:avoid-msg-edit-crash-on-deleted-messages into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403218 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:avoid-msg-edit-crash-on-deleted-messages. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-ui-list-revisions into launchpad:master
The proposal to merge ~pappacena/launchpad:comment-editing-ui-list-revisions into launchpad:master has been updated. Description changed to: Some screenshots: - https://monosnap.com/file/au1QEPw7qb40DUMWaAUOE6c3N4QEGU - https://monosnap.com/file/ENRLcSbY51NObjAAq3aJHH8WEm2tg4 - https://monosnap.com/file/PG2bG2UCBqn6BuOVlbffpeXWkwuW33 For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403213 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:comment-editing-ui-list-revisions into launchpad:master. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:avoid-msg-edit-crash-on-deleted-messages into launchpad:master
Thiago F. Pappacena has proposed merging
~pappacena/launchpad:avoid-msg-edit-crash-on-deleted-messages into
launchpad:master.
Commit message:
Avoiding msg edit JS crash when displaying a msg without content
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403218
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~pappacena/launchpad:avoid-msg-edit-crash-on-deleted-messages into
launchpad:master.
diff --git a/lib/lp/services/messages/javascript/messages.edit.js b/lib/lp/services/messages/javascript/messages.edit.js
index 3c4c0e5..a1e6806 100644
--- a/lib/lp/services/messages/javascript/messages.edit.js
+++ b/lib/lp/services/messages/javascript/messages.edit.js
@@ -181,6 +181,11 @@ YUI.add('lp.services.messages.edit', function(Y) {
"cancel_btn": container.one('.editable-message-cancel-btn'),
"last_edit": container.one('.editable-message-last-edit-date')
};
+// If the msg body or the msg form are not defined, don't try to do
+// anything else.
+if (!elements.msg_form || !elements.msg_body) {
+return;
+}
elements.textarea = elements.msg_form.one('textarea');
module.hideEditMessageField(elements.msg_body, elements.msg_form);
diff --git a/lib/lp/services/messages/javascript/tests/test_messages.edit.html b/lib/lp/services/messages/javascript/tests/test_messages.edit.html
index 94c60ea..1ef776f 100644
--- a/lib/lp/services/messages/javascript/tests/test_messages.edit.html
+++ b/lib/lp/services/messages/javascript/tests/test_messages.edit.html
@@ -91,5 +91,14 @@ GNU Affero General Public License version 3 (see the file LICENSE).
+
+
+
+Deleted msg.
+
+
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-ui-list-revisions into launchpad:master
Thiago F. Pappacena has proposed merging
~pappacena/launchpad:comment-editing-ui-list-revisions into launchpad:master.
Commit message:
Showing message revision list in the UI
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403213
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~pappacena/launchpad:comment-editing-ui-list-revisions into launchpad:master.
diff --git a/lib/canonical/launchpad/icing/css/base.scss b/lib/canonical/launchpad/icing/css/base.scss
index 6ee2779..1e33ca7 100644
--- a/lib/canonical/launchpad/icing/css/base.scss
+++ b/lib/canonical/launchpad/icing/css/base.scss
@@ -579,6 +579,61 @@ body {
margin: 5px;
}
}
+
+.message-revision-container {
+position: absolute;
+background-color: white;
+margin-top: 20px;
+display: none;
+border: 1px solid #ddd;
+width: 45em;
+z-index: 100;
+max-height: 200px;
+overflow-y: auto;
+overflow-x: hidden;
+
+.message-revision-container-header {
+background-color: #eee;
+border-bottom: 1px solid #ddd;
+padding: 10px;
+
+span {
+text-align: left;
+display: inline-block;
+font-size: 110%;
+font-weight: bold;
+}
+
+img {
+float: right;
+cursor: pointer;
+}
+}
+
+.message-revision-item {
+border-bottom: 1px solid #ddd;
+padding: 2px;
+
+.message-revision-title {
+padding: 5px;
+cursor: pointer;
+font-weight: 300;
+}
+
+.message-revision-body {
+display: none;
+padding-left: 20px;
+padding-bottom: 10px;
+}
+}
+
+.active {
+background-color: #eee;
+.message-revision-title {
+font-weight: bold;
+}
+}
+}
}
@import 'typography',
diff --git a/lib/lp/answers/browser/question.py b/lib/lp/answers/browser/question.py
index 9d4986d..52cff40 100644
--- a/lib/lp/answers/browser/question.py
+++ b/lib/lp/answers/browser/question.py
@@ -1195,13 +1195,11 @@ class QuestionMessageDisplayView(LaunchpadView):
return check_permission('launchpad.Moderate', self.context)
def getBoardCommentCSSClass(self):
-css_classes = ["boardComment"]
+css_classes = ["boardComment", "editable-message"]
if not self.context.visible:
# If a comment that isn't visible is being rendered, it's being
# rendered for an admin or registry_expert.
css_classes.append("adminHiddenComment")
-if self.can_edit:
-css_classes.append("editable-message")
return " ".join(css_classes)
@property
diff --git a/lib/lp/answers/stories/question-workflow.txt b/lib/lp/answers/stories/question-workflow.txt
index 5e39acd..1a0b67a 100755
--- a/lib/lp/answers/stories/question-workflow.txt
+++ b/lib/lp/answers/stories/question-workflow.txt
@@ -215,7 +215,7 @@ The confirmed answer is also highlighted.
>>> soup = find_main_content(owner_browser.contents)
>>> bestAnswer = soup.find_all('div', 'boardComment')[-2]
->>> print(bestAnswer.find('img'))
+>>> print(bestAnswer.find_all('img')[1])
>>> print(soup.find(
diff --git a/lib/lp/answers/templates/questionmessage-display.pt b/lib/lp/answers/templates/questionmessage-display.pt
index fc3651c..c1ed29a 100644
--- a/lib/lp/answers/templates/questionmessage-display.pt
+++ b/lib/lp/answers/templates/questionmessage-display.pt
@@ -14,6 +14,25 @@
+
+
+ Revision history for this message
+
+
+
+ <div class='message-revision-item'>
+<div class='message-revision-title'>
+<a class="js-action">
+Revision #{revision}, created at {date_created}
+</a>
+</div>
+<div class='message-revision-body'>{content}</div>
+ </div>
+
+
+
+
@@ -28,11 +47,12 @@
datetime context/datecreated/fmt:isodate"
tal:content="context/datecreated/fmt:displaydate">Thursday 13:21
-(last edit (last edit ):
+tal:content="context/date_last_edited/fmt:displaydate" />):
diff --git a/lib/lp/services/messages/interfaces/messagerevision.py b/lib/lp/services/messages/interfaces/messagerevision.py
index 3ee5a9
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:fix-distroseries-diff-comment into launchpad:master
The proposal to merge ~pappacena/launchpad:fix-distroseries-diff-comment into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403202 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:fix-distroseries-diff-comment. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:fix-distroseries-diff-comment into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:fix-distroseries-diff-comment into launchpad:master. Commit message: Fixing a bug on DistroSeries diff page Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403202 DistroSeriesDifferenceDisplayComment object should follow the same pattern used by BugComment, for example: implement IDistroSeriesDifferenceDisplayComment, but delegating anything related to IMessage to comment.message. -- Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:fix-distroseries-diff-comment into launchpad:master. diff --git a/lib/lp/registry/browser/distroseriesdifference.py b/lib/lp/registry/browser/distroseriesdifference.py index d58ce5f..60f7b87 100644 --- a/lib/lp/registry/browser/distroseriesdifference.py +++ b/lib/lp/registry/browser/distroseriesdifference.py @@ -1,4 +1,4 @@ -# Copyright 2010-2018 Canonical Ltd. This software is licensed under the +# Copyright 2010-2021 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). """Browser views for DistroSeriesDifferences.""" @@ -9,6 +9,7 @@ __all__ = [ 'DistroSeriesDifferenceView', ] +from lazr.delegates import delegate_to from lazr.restful.interfaces import IWebServiceClientRequest from zope.browserpage import ViewPageTemplateFile from zope.component import ( @@ -46,6 +47,7 @@ from lp.services.comments.interfaces.conversation import ( IComment, IConversation, ) +from lp.services.messages.interfaces.message import IMessage from lp.services.propertycache import cachedproperty from lp.services.webapp import ( LaunchpadView, @@ -240,11 +242,12 @@ class DistroSeriesDifferenceView(LaunchpadFormView): self.show_package_diffs_request_link) -class IDistroSeriesDifferenceDisplayComment(IComment): +class IDistroSeriesDifferenceDisplayComment(IComment, IMessage): """Marker interface.""" @implementer(IDistroSeriesDifferenceDisplayComment) +@delegate_to(IMessage, context='_message') class DistroSeriesDifferenceDisplayComment(MessageComment): """Used simply to provide `IComment` for rendering.""" @@ -258,6 +261,7 @@ class DistroSeriesDifferenceDisplayComment(MessageComment): """Setup the attributes required by `IComment`.""" super(DistroSeriesDifferenceDisplayComment, self).__init__(None) self.comment = comment +self._message = comment.message def get_message(comment): ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-ui-codereview into launchpad:master
The proposal to merge ~pappacena/launchpad:comment-editing-ui-codereview into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/402850 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-ui-bugcomment. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-ui-bugcomment into launchpad:master
The proposal to merge ~pappacena/launchpad:comment-editing-ui-bugcomment into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/402604 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-ui. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-ui into launchpad:master
The proposal to merge ~pappacena/launchpad:comment-editing-ui into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/402522 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-revisions-api. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:prevent-admins-from-editing-codereview-comment into launchpad:master
The proposal to merge ~pappacena/launchpad:prevent-admins-from-editing-codereview-comment into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403146 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:prevent-admins-from-editing-codereview-comment. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-ui-codereview into launchpad:master
This should be good to go now. Diff comments: > diff --git a/lib/lp/code/templates/codereviewcomment-header.pt > b/lib/lp/code/templates/codereviewcomment-header.pt > index efd37eb..254e764 100644 > --- a/lib/lp/code/templates/codereviewcomment-header.pt > +++ b/lib/lp/code/templates/codereviewcomment-header.pt > @@ -18,7 +18,15 @@ > datetime context/comment_date/fmt:isodate" >tal:content="context/comment_date/fmt:displaydate"> >7 minutes ago > -: > + tal:condition="not:context/date_last_edited">: Ok! Applying here the same that was done in the other MP. > + > + (last edit + itemprop="editTime" > + tal:attributes="title > context/date_last_edited/fmt:datetime; > +datetime context/date_last_edited/fmt:isodate" > + tal:content="context/date_last_edited/fmt:displaydate" />): > + > + > tal:condition="context/from_superseded" >class="sprite warning-icon" -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/402850 Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-ui-bugcomment. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-ui-bugcomment into launchpad:master
Pushed the requested change. This should be good to go. Diff comments: > diff --git a/lib/lp/bugs/templates/bugcomment-box.pt > b/lib/lp/bugs/templates/bugcomment-box.pt > index 1b087d7..35c3c10 100644 > --- a/lib/lp/bugs/templates/bugcomment-box.pt > +++ b/lib/lp/bugs/templates/bugcomment-box.pt > @@ -47,13 +56,26 @@ > datetime comment/datecreated/fmt:isodate" >tal:content="comment/datecreated/fmt:displaydate"> >7 minutes ago > - : > + tal:condition="not:context/date_last_edited">: Applying here the same done for the other MP. > + > + (last edit + itemprop="editTime" > + tal:attributes="title > context/date_last_edited/fmt:datetime; > +datetime context/date_last_edited/fmt:isodate" > + tal:content="context/date_last_edited/fmt:displaydate" > />): > + > + > >tal:content="comment/title" /> > > > > + > + + tal:condition="view/can_edit"/> > + > + > > tal:attributes="href comment/fmt:url" -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/402604 Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-ui. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-ui into launchpad:master
Pushed the requested changes. This should be good to go now.
Diff comments:
> diff --git a/lib/lp/answers/templates/questionmessage-display.pt
> b/lib/lp/answers/templates/questionmessage-display.pt
> index b69b4b6..e41a0a7 100644
> --- a/lib/lp/answers/templates/questionmessage-display.pt
> +++ b/lib/lp/answers/templates/questionmessage-display.pt
> @@ -5,9 +5,10 @@
> itemscope=""
>itemtype="http://schema.org/UserComments;
> - tal:define="css_classes view/getBoardCommentCSSClass"
> + tal:define="css_classes python: view.getBoardCommentCSSClass()"
Yep. I was concatenating something here, but ended up moving it into the
method. Let me revert this piece.
>tal:attributes="class string:${css_classes};
> - id string:comment-${context/index}">
> + id string:comment-${context/index};
> + data-baseurl context/fmt:url">
>
>
>
> @@ -25,8 +26,20 @@
>itemprop="commentTime"
>tal:attributes="title context/datecreated/fmt:datetime;
> datetime context/datecreated/fmt:isodate"
> - tal:content="context/datecreated/fmt:displaydate">Thursday
> -13:21:
> + tal:content="context/datecreated/fmt:displaydate">Thursday 13:21
> + tal:condition="not:context/date_last_edited">:
Taking care of the white spaces makes it a bit hard, but I think I've found a
reasonable way. Changing it.
> +
> + (last edit + itemprop="editTime"
> + tal:attributes="title context/date_last_edited/fmt:datetime;
> +datetime context/date_last_edited/fmt:isodate"
> + tal:content="context/date_last_edited/fmt:displaydate" />):
> +
> +
> +
> +
> + + tal:condition="view/can_edit"/>
>
>
> diff --git a/lib/lp/services/messages/javascript/messages.edit.js
> b/lib/lp/services/messages/javascript/messages.edit.js
> new file mode 100644
> index 000..5cb6156
> --- /dev/null
> +++ b/lib/lp/services/messages/javascript/messages.edit.js
> @@ -0,0 +1,208 @@
> +/* Copyright 2015-2021 Canonical Ltd. This software is licensed under the
> + * GNU Affero General Public License version 3 (see the file LICENSE).
> + *
> + * This modules controls HTML comments in order to make them editable. To do
> + * so, it requires:
> + * - A div container with the class .editable-message containing everything
> + * else related to the message
> + * - A data-baseurl="/path/to/msg" on the .editable-message container
> + * - A .editable-message-body container with the original msg content
> + * - A .editable-message-edit-btn element inside the main container, that
> will
> + * switch the view to edit form when clicked.
> + * - A .editable-message-form, with a textarea and 2 buttons:
> + * .editable-message-update-btn and .editable-message-cancel-btn.
> + * - A .editable-message-last-edit-date span, where we update the date of
> the
> + * last message editing.
> + *
> + * Once those HTML elements are available in the page, this module should be
> + * initialized with `lp.services.messages.edit.setup()`.
> + *
> + * @module Y.lp.services.messages.edit
> + * @requires node, DOM, lp.client
> + */
> +YUI.add('lp.services.messages.edit', function(Y) {
> +var module = Y.namespace('lp.services.messages.edit');
> +
> +module.msg_edit_success_notification = (
> +"Message edited, but the original content may still be publicly " +
> +"visible using the API.Please, " +
Ok!
> +"" +
> +"check the API documentation in case " +
Sure!
> +"need to remove old message revisions."
> +);
> +module.msg_edit_error_notification = (
> +"There was an error updating the comment. " +
> +"Please, try again in some minutes."
Ok!
> +);
> +
> +module.htmlify_msg = function(text) {
Right!
> +text = text.replace(/ +text = text.replace(/>/g, "");
> +text = text.replace(/\n/g, "");
> +return "" + text+ "";
> +};
> +
> +module.showEditMessageField = function(msg_body, msg_form) {
> +msg_body.setStyle('display', 'none');
> +msg_form.setStyle('display', 'block');
> +};
> +
> +module.hideEditMessageField = function(msg_body, msg_form) {
> +msg_body.setStyle('display', 'block');
> +msg_form.setStyle('display', 'none');
> +};
> +
> +module.saveMessageContent = function(
> +msg_path, new_content, on_success, on_failure) {
> +var msg_url = "/api/devel" + msg_path;
> +var config = {
> +on: {
> + success: on_success,
> + failure: on_failure
> + },
> +parameters: {"new_content": new_content}
> +};
> +this.lp_client.named_post(msg_url, 'editContent', config);
> +};
> +
> +module.showNotification = function(container, msg, can_dismiss) {
> +
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:prevent-admins-from-editing-codereview-comment into launchpad:master
Pushed the permission name change Diff comments: > diff --git a/lib/lp/code/configure.zcml b/lib/lp/code/configure.zcml > index a373286..7a019ed 100644 > --- a/lib/lp/code/configure.zcml > +++ b/lib/lp/code/configure.zcml > @@ -548,7 +548,7 @@ > > > > - + > interface="lp.services.messages.interfaces.message.IMessageEdit" /> > interface="lp.code.interfaces.codereviewcomment.ICodeReviewCommentView"/> > -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403146 Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:prevent-admins-from-editing-codereview-comment. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:prevent-admins-from-editing-codereview-comment into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:prevent-admins-from-editing-codereview-comment into launchpad:master. Commit message: Make sure that only owners can edit CodeReviewComment by requiring a more specific permission Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403146 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:prevent-admins-from-editing-codereview-comment into launchpad:master. diff --git a/lib/lp/code/configure.zcml b/lib/lp/code/configure.zcml index a373286..7a019ed 100644 --- a/lib/lp/code/configure.zcml +++ b/lib/lp/code/configure.zcml @@ -548,7 +548,7 @@ - diff --git a/lib/lp/security.py b/lib/lp/security.py index 1eeca4e..e5b633f 100644 --- a/lib/lp/security.py +++ b/lib/lp/security.py @@ -2636,6 +2636,15 @@ class CodeReviewCommentView(DelegatedAuthorization): obj, obj.branch_merge_proposal) +class CodeReviewCommentDrive(AuthorizationBase): +permission = 'launchpad.Driver' +usedfor = ICodeReviewComment + +def checkAuthenticated(self, user): +"""Only message owner can edit its content.""" +return user.isOwner(self.obj) + + class CodeReviewCommentDelete(DelegatedAuthorization): permission = 'launchpad.Edit' usedfor = ICodeReviewCommentDeletion diff --git a/lib/lp/services/messages/tests/test_message.py b/lib/lp/services/messages/tests/test_message.py index c483305..bd4fc12 100644 --- a/lib/lp/services/messages/tests/test_message.py +++ b/lib/lp/services/messages/tests/test_message.py @@ -360,9 +360,9 @@ class TestMessageEditingAPI(MessageTypeScenariosMixin, TestCaseWithFactory): self.assertIsNone(edited_obj["date_deleted"]) self.assertIsNotNone(edited_obj["date_last_edited"]) -def test_edit_message_permission_denied_for_non_owner(self): +def assertPermissionDeniedEditMessage(self, caller_person): msg = self.makeMessage(content="initial content") -ws = self.getWebservice(self.factory.makePerson()) +ws = self.getWebservice(caller_person) url = self.getMessageAPIURL(msg) response = ws.named_post( url, 'editContent', new_content="the new content") @@ -373,6 +373,13 @@ class TestMessageEditingAPI(MessageTypeScenariosMixin, TestCaseWithFactory): self.assertIsNone(edited_obj["date_deleted"]) self.assertIsNone(edited_obj["date_last_edited"]) +def test_edit_message_permission_denied_for_non_owner(self): +self.assertPermissionDeniedEditMessage(self.factory.makePerson()) + +def test_edit_message_permission_denied_for_admin(self): +self.assertPermissionDeniedEditMessage( +self.factory.makeAdministrator()) + def test_delete_message(self): msg = self.makeMessage(content="initial content") ws = self.getWebservice(self.person) ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:prevent-admins-from-editing-codereview-comment into launchpad:master
The proposal to merge ~pappacena/launchpad:prevent-admins-from-editing-codereview-comment into launchpad:master has been updated. Status: Needs review => Work in progress For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403146 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:prevent-admins-from-editing-codereview-comment into launchpad:master. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:prevent-admins-from-editing-codereview-comment into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:prevent-admins-from-editing-codereview-comment into launchpad:master. Commit message: Make sure that only owners can edit CodeReviewComment Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403146 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:prevent-admins-from-editing-codereview-comment into launchpad:master. diff --git a/lib/lp/security.py b/lib/lp/security.py index 1eeca4e..9b1cfa8 100644 --- a/lib/lp/security.py +++ b/lib/lp/security.py @@ -2636,6 +2636,15 @@ class CodeReviewCommentView(DelegatedAuthorization): obj, obj.branch_merge_proposal) +class CodeReviewCommentEdit(AuthorizationBase): +permission = 'launchpad.Edit' +usedfor = ICodeReviewComment + +def checkAuthenticated(self, user): +"""Only message owner can edit it.""" +return user.isOwner(self.obj) + + class CodeReviewCommentDelete(DelegatedAuthorization): permission = 'launchpad.Edit' usedfor = ICodeReviewCommentDeletion diff --git a/lib/lp/services/messages/tests/test_message.py b/lib/lp/services/messages/tests/test_message.py index c483305..bd4fc12 100644 --- a/lib/lp/services/messages/tests/test_message.py +++ b/lib/lp/services/messages/tests/test_message.py @@ -360,9 +360,9 @@ class TestMessageEditingAPI(MessageTypeScenariosMixin, TestCaseWithFactory): self.assertIsNone(edited_obj["date_deleted"]) self.assertIsNotNone(edited_obj["date_last_edited"]) -def test_edit_message_permission_denied_for_non_owner(self): +def assertPermissionDeniedEditMessage(self, caller_person): msg = self.makeMessage(content="initial content") -ws = self.getWebservice(self.factory.makePerson()) +ws = self.getWebservice(caller_person) url = self.getMessageAPIURL(msg) response = ws.named_post( url, 'editContent', new_content="the new content") @@ -373,6 +373,13 @@ class TestMessageEditingAPI(MessageTypeScenariosMixin, TestCaseWithFactory): self.assertIsNone(edited_obj["date_deleted"]) self.assertIsNone(edited_obj["date_last_edited"]) +def test_edit_message_permission_denied_for_non_owner(self): +self.assertPermissionDeniedEditMessage(self.factory.makePerson()) + +def test_edit_message_permission_denied_for_admin(self): +self.assertPermissionDeniedEditMessage( +self.factory.makeAdministrator()) + def test_delete_message(self): msg = self.makeMessage(content="initial content") ws = self.getWebservice(self.person) ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-revisions-api into launchpad:master
The proposal to merge ~pappacena/launchpad:comment-editing-revisions-api into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/402285 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-api. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-revisions-api into launchpad:master
The proposal to merge ~pappacena/launchpad:comment-editing-revisions-api into launchpad:master has been updated. Status: Approved => Needs review For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/402285 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-api. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-revisions-api into launchpad:master
The proposal to merge ~pappacena/launchpad:comment-editing-revisions-api into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/402285 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-api. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-revisions-api into launchpad:master
The proposal to merge ~pappacena/launchpad:comment-editing-revisions-api into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/402285 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-api. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-api into launchpad:master
The proposal to merge ~pappacena/launchpad:comment-editing-api into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/402211 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-model. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-api into launchpad:master
Pushed the requested change. This should be good to land now. Diff comments: > diff --git a/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt > b/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt > index 32f0373..58b86e7 100644 > --- a/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt > +++ b/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt > @@ -198,9 +198,13 @@ The comments on a branch merge proposal are exposed > through the API. > as_quoted_email: '> This is great work' > author_link: 'http://api.launchpad.test/devel/~...' > branch_merge_proposal_link: 'http://.../~source/fooix/fix-it/+merge/...' > +content: 'This is great work' I agree. Changing the description of `ICodeReviewComment.message_body` to "Deprecated. Use "content" attribute instead.". > date_created: '...' > +date_deleted: None > +date_last_edited: None > id: ... > message_body: 'This is great work' > +owner_link: 'http://...' > resource_type_link: 'http://.../#code_review_comment' > self_link: 'http://.../~source/fooix/fix-it/+merge/.../comments/...' > title: 'Comment on proposed merge of lp://dev/~source/fooix/fix-it into > lp://dev/~target/fooix/trunk' > diff --git a/lib/lp/services/messages/browser/message.py > b/lib/lp/services/messages/browser/message.py > index 3b5a3c4..06800e6 100644 > --- a/lib/lp/services/messages/browser/message.py > +++ b/lib/lp/services/messages/browser/message.py > @@ -18,7 +18,7 @@ class QuestionMessageCanonicalUrlData: > > def __init__(self, question, message): > self.inside = question > -self.path = "messages/%d" % list(question.messages).index(message) > +self.path = "messages/%d" % message.display_index Ok! > > > @implementer(ICanonicalUrlData) -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/402211 Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-model. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-model into launchpad:master
The proposal to merge ~pappacena/launchpad:comment-editing-model into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401894 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-model. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-model into launchpad:master
Pushed the requested changes. This should be good to go now.
Diff comments:
> diff --git a/lib/lp/services/messages/interfaces/messagerevision.py
> b/lib/lp/services/messages/interfaces/messagerevision.py
> new file mode 100644
> index 000..3b72673
> --- /dev/null
> +++ b/lib/lp/services/messages/interfaces/messagerevision.py
> @@ -0,0 +1,69 @@
> +# Copyright 2019-2021 Canonical Ltd. This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Message revision history."""
> +
> +from __future__ import absolute_import, print_function, unicode_literals
> +
> +__all__ = [
> +'IMessageRevision',
> +'IMessageRevisionChunk',
> +]
> +
> +from lazr.restful.fields import Reference
> +from zope.interface import (
> +Attribute,
> +Interface,
> +)
> +from zope.schema import (
> +Datetime,
> +Int,
> +Text,
> +)
> +
> +from lp import _
> +from lp.services.messages.interfaces.message import IMessage
> +
> +
> +class IMessageRevisionView(Interface):
> +"""IMessageRevision readable attributes."""
> +id = Int(title=_("ID"), required=True, readonly=True)
> +
> +revision = Int(title=_("Revision number"), required=True, readonly=True)
> +
> +content = Text(
> +title=_("The message at the given revision"),
> +required=False, readonly=True)
:facepalm:
Fixing it.
> +
> +message = Reference(
> +title=_('The current message of this revision.'),
> +schema=IMessage, required=True, readonly=True)
> +
> +date_created = Datetime(
> +title=_("The time when this message revision was created."),
> +required=True, readonly=True)
> +
> +date_deleted = Datetime(
> +title=_("The time when this message revision was created."),
> +required=False, readonly=True)
> +
> +chunks = Attribute(_('Message pieces'))
> +
> +
> +class IMessageRevisionEdit(Interface):
> +"""IMessageRevision editable attributes."""
> +
> +def deleteContent():
> +"""Logically deletes this MessageRevision."""
> +
> +
> +class IMessageRevision(IMessageRevisionView, IMessageRevisionEdit):
> +"""A historical revision of a IMessage."""
> +
> +
> +class IMessageRevisionChunk(Interface):
> +id = Int(title=_('ID'), required=True, readonly=True)
> +messagerevision = Int(
> +title=_('MessageRevision'), required=True, readonly=True)
> +sequence = Int(title=_('Sequence order'), required=True, readonly=True)
> +content = Text(title=_('Text content'), required=False, readonly=True)
> diff --git a/lib/lp/services/messages/model/message.py
> b/lib/lp/services/messages/model/message.py
> index e592daa..b73e212 100644
> --- a/lib/lp/services/messages/model/message.py
> +++ b/lib/lp/services/messages/model/message.py
> @@ -164,6 +176,63 @@ class Message(SQLBase):
> """See `IMessage`."""
> return None
>
> +@cachedproperty
> +def revisions(self):
> +"""See `IMessage`."""
> +return list(Store.of(self).find(
> +MessageRevision,
> +MessageRevision.message == self
> +).order_by(Desc(MessageRevision.revision)))
> +
> +def editContent(self, new_content):
> +"""See `IMessage`."""
> +store = Store.of(self)
> +
> +# Move the old content to a new revision.
> +date_created = (
> +self.date_last_edited if self.date_last_edited is not None
> +else self.datecreated)
> +current_rev_num = store.find(
> +Max(MessageRevision.revision),
> +MessageRevision.message == self).one()
> +rev_num = (current_rev_num or 0) + 1
> +rev = MessageRevision(
> +message=self, revision=rev_num, date_created=date_created)
> +self.date_last_edited = utc_now()
> +store.add(rev)
> +
> +# Move the current text content to the recently created revision.
> +for chunk in self._chunks:
> +if chunk.blob is None:
> +revision_chunk = MessageRevisionChunk(
> +rev, chunk.sequence, chunk.content)
> +store.add(revision_chunk)
> +store.remove(chunk)
> +
> +# Create the new content.
> +new_chunk = MessageChunk(message=self, sequence=1,
> content=new_content)
Ok! I'll try to fill eventual gaps instead of jumping directly into max_seq + 1.
> +store.add(new_chunk)
> +
> +store.flush()
> +
> +# Clean up caches.
> +del get_property_cache(self).text_contents
> +del get_property_cache(self).chunks
> +del get_property_cache(self).revisions
> +return rev
> +
> +def deleteContent(self):
> +"""See `IMessage`."""
> +store = Store.of(self)
> +store.find(MessageChunk, MessageChunk.message == self).remove()
> +for rev in self.revisions:
> +store.find(MessageRevisionChunk,
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:merge-db-stable into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:merge-db-stable into launchpad:master. Commit message: Merge db-stable 6d412dafb1959d2eef85de03c10e924666d2fa51 (MessageRevision and MessageRevisionChunk tables, and adding columns for last edit and delete date on Message) Requested reviews: Thiago F. Pappacena (pappacena) For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403066 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:merge-db-stable. diff --git a/database/schema/patch-2210-31-0.sql b/database/schema/patch-2210-31-0.sql new file mode 100644 index 000..8c11aac --- /dev/null +++ b/database/schema/patch-2210-31-0.sql @@ -0,0 +1,50 @@ +-- Copyright 2021 Canonical Ltd. This software is licensed under the +-- GNU Affero General Public License version 3 (see the file LICENSE). + +SET client_min_messages=ERROR; + +ALTER TABLE Message +ADD COLUMN date_deleted timestamp without time zone, +ADD COLUMN date_last_edited timestamp without time zone; + +CREATE TABLE MessageRevision ( +id serial PRIMARY KEY, +message integer NOT NULL REFERENCES Message, +revision integer NOT NULL, +subject text, +date_created timestamp without time zone NOT NULL, +date_deleted timestamp without time zone +) WITH (fillfactor='100'); + +CREATE UNIQUE INDEX messagerevision__message__revision__key +ON MessageRevision(message, revision); + +COMMENT ON TABLE MessageRevision IS 'Old versions of an edited Message'; +COMMENT ON COLUMN MessageRevision.message +IS 'The current message of this revision'; +COMMENT ON COLUMN MessageRevision.revision +IS 'The revision monotonic increasing number'; +COMMENT ON COLUMN MessageRevision.date_created +IS 'When the original message was edited and created this revision'; +COMMENT ON COLUMN MessageRevision.date_deleted +IS 'If this revision was deleted, when did that happen'; + + +CREATE TABLE MessageRevisionChunk ( +id serial PRIMARY KEY, +messagerevision integer NOT NULL REFERENCES MessageRevision, +sequence integer NOT NULL, +content text NOT NULL +) WITH (fillfactor='100'); + +CREATE UNIQUE INDEX messagerevisionchunk__messagerevision__sequence__key +ON MessageRevisionChunk(messagerevision, sequence); + +COMMENT ON TABLE MessageRevisionChunk +IS 'Old chunks of a message when a revision was created for it'; +COMMENT ON COLUMN MessageRevisionChunk.sequence +IS 'Order of this particular chunk'; +COMMENT ON COLUMN MessageRevisionChunk.content +IS 'Text content for this chunk of the message.'; + +INSERT INTO LaunchpadDatabaseRevision VALUES (2210, 31, 0); ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:merge-db-stable into launchpad:master
The proposal to merge ~pappacena/launchpad:merge-db-stable into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403066 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:merge-db-stable. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:merge-db-stable into launchpad:master
Review: Approve Deployed to the production database today. -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/403066 Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:merge-db-stable. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-db-patch into launchpad:db-devel
The proposal to merge ~pappacena/launchpad:comment-editing-db-patch into launchpad:db-devel has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401976 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-db-patch. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-revisions-api into launchpad:master
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))
Fixing it
> +
> +
> 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))
Fixing it
> +
> +
> 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))
Fixing it
> +
> +
> 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."),
Right. Changed it along the way, and forgot to update this comment.
Fixing. Thanks!
> +# 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)
I'll make it a bit more specific, with `schema=IMessage` (that BugComment,
QuestionMessage and CodeReviewComment implements).
> +
> +date_created = exported(Datetime(
> title=_("The time when this message revision was created."),
> -required=True, readonly=True)
> +required=True,
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-api into launchpad:master
Addressed all the comments. I would like your opinion on the comment on
xx-branchmergeproposal.txt, about deprecating an API attribute.
Diff comments:
> diff --git a/lib/lp/answers/browser/question.py
> b/lib/lp/answers/browser/question.py
> index b039552..757da13 100644
> --- a/lib/lp/answers/browser/question.py
> +++ b/lib/lp/answers/browser/question.py
> @@ -257,6 +258,17 @@ class QuestionSetNavigation(Navigation):
> canonical_url(question, self.request), status=301)
>
>
> +class QuestionNavigation(Navigation):
> +"""Navigation for the IQuestion."""
> +
> +usedfor = IQuestion
> +
> +@stepthrough('messages')
> +def traverse_comments(self, index):
Right!
> +index = int(index) - 1
> +return self.context.messages[index]
Right! Fixing it.
> +
> +
> 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 2b61552..e40967f 100644
> --- a/lib/lp/bugs/browser/bugcomment.py
> +++ b/lib/lp/bugs/browser/bugcomment.py
> @@ -72,7 +72,10 @@ def build_comments_from_chunks(
> cache = get_property_cache(message)
> if getattr(cache, 'chunks', None) is None:
> cache.chunks = []
> -cache.chunks.append(removeSecurityProxy(chunk))
> +# Soft-deleted messages will have None chunk here. Skip cache
> +# filling in this case.
> +if chunk is not None:
This method is used by `get_comments_for_bugtask`, which is the one used to
traverse bugtask to its comments. If we filter out completely (with an `INNER
JOIN`, `MessageChunk.id != None`) the Message objects that doesn't have
content, we wouldn't be able to fetch "deleted" bug messages in the API, for
example.
And the initial idea was keeping both in the web UI and in the API the Message
objects that had its contents deleted.
> +cache.chunks.append(removeSecurityProxy(chunk))
> bug_comment = comments.get(message.id)
> if bug_comment is None:
> if bugmessage.index == 0 and hide_first:
> diff --git a/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt
> b/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt
> index 32f0373..58b86e7 100644
> --- a/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt
> +++ b/lib/lp/code/stories/webservice/xx-branchmergeproposal.txt
> @@ -198,9 +198,13 @@ The comments on a branch merge proposal are exposed
> through the API.
> as_quoted_email: '> This is great work'
> author_link: 'http://api.launchpad.test/devel/~...'
> branch_merge_proposal_link: 'http://.../~source/fooix/fix-it/+merge/...'
> +content: 'This is great work'
Ideally, deprecating `message_body` (keeping all the messages compatible with
the `content` interface) would be preferred, but I'm a bit afraid of breaking
backward compatibility with whatever currently depends on this API.
Do you think we should do it in this MP?
> date_created: '...'
> +date_deleted: None
> +date_last_edited: None
> id: ...
> message_body: 'This is great work'
> +owner_link: 'http://...'
> resource_type_link: 'http://.../#code_review_comment'
> self_link: 'http://.../~source/fooix/fix-it/+merge/.../comments/...'
> title: 'Comment on proposed merge of lp://dev/~source/fooix/fix-it into
> lp://dev/~target/fooix/trunk'
> diff --git a/lib/lp/services/messages/browser/message.py
> b/lib/lp/services/messages/browser/message.py
> index 3b5a3c4..06800e6 100644
> --- a/lib/lp/services/messages/browser/message.py
> +++ b/lib/lp/services/messages/browser/message.py
> @@ -18,7 +18,7 @@ class QuestionMessageCanonicalUrlData:
>
> def __init__(self, question, message):
> self.inside = question
> -self.path = "messages/%d" % list(question.messages).index(message)
> +self.path = "messages/%d" % message.display_index
Actually, the current traverse for a specific question message doesn't seem to
work today:
For example:
- https://answers.launchpad.net/api/devel/launchpad/+question/697126/messages
returns a list of messages
-
https://answers.launchpad.net/api/devel/launchpad/+question/697126/messages/{0,1}
both returns error
Also, the answers/browser/configure.zcml declares
`path_expression="string:messages/${display_index}"`, so I guess the initial
intention was always to use "display_index" (starting from 1), as for code
review comments and bug comments.
>
>
> @implementer(ICanonicalUrlData)
> diff --git a/lib/lp/services/messages/interfaces/message.py
> b/lib/lp/services/messages/interfaces/message.py
> index 70d50b7..73edea1 100644
> --- a/lib/lp/services/messages/interfaces/message.py
> +++ b/lib/lp/services/messages/interfaces/message.py
> @@ -51,44 +56,62 @@ from lp.services.webservice.apihelpers import
> patch_reference_property
>
> class IMessageEdit(Interface):
>
> +
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-db-patch into launchpad:db-devel
Pushed the requested changes. This should be ready to be merged now. Diff comments: > diff --git a/database/schema/patch-2210-31-0.sql > b/database/schema/patch-2210-31-0.sql > new file mode 100644 > index 000..3a15c86 > --- /dev/null > +++ b/database/schema/patch-2210-31-0.sql > @@ -0,0 +1,47 @@ > +-- Copyright 2021 Canonical Ltd. This software is licensed under the > +-- GNU Affero General Public License version 3 (see the file LICENSE). > + > +SET client_min_messages=ERROR; > + > +ALTER TABLE Message > +ADD COLUMN date_deleted timestamp without time zone, > +ADD COLUMN date_last_edited timestamp without time zone; > + > +CREATE TABLE MessageRevision ( > +id serial PRIMARY KEY, > +message integer NOT NULL REFERENCES Message, > +subject text, > +revision integer NOT NULL, Sure! > +date_created timestamp without time zone NOT NULL, > +date_deleted timestamp without time zone > +) WITH (fillfactor='100'); > + > +CREATE UNIQUE INDEX messagerevision__message__revision__key > +ON MessageRevision(message, revision); > + > +COMMENT ON TABLE MessageRevision IS 'Old versions of an edited Message'; > +COMMENT ON COLUMN MessageRevision.message > +IS 'The current message of this revision'; > +COMMENT ON COLUMN MessageRevision.revision > +IS 'The revision monotonic increasing number'; > +COMMENT ON COLUMN MessageRevision.date_created > +IS 'When the original message was edited and created this revision'; > +COMMENT ON COLUMN MessageRevision.date_deleted > +IS 'If this revision was deleted, when did that happen'; > + > + > +CREATE TABLE MessageRevisionChunk ( > +id serial PRIMARY KEY, > +messagerevision integer NOT NULL REFERENCES MessageRevision, > +sequence integer NOT NULL, > +content text NOT NULL > +) WITH (fillfactor='100'); It makes sense. Adding UNIQUE (messagerevision, sequence). > + > +COMMENT ON TABLE MessageRevisionChunk > +IS 'Old chunks of a message when a revision was created for it'; > +COMMENT ON COLUMN MessageRevisionChunk.sequence > +IS 'Order of this particular chunk'; > +COMMENT ON COLUMN MessageRevisionChunk.content > +IS 'Text content for this chunk of the message.'; > + > +INSERT INTO LaunchpadDatabaseRevision VALUES (2210, 31, 0); -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401976 Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-db-patch. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-ui-codereview into launchpad:master
Thiago F. Pappacena has proposed merging
~pappacena/launchpad:comment-editing-ui-codereview into launchpad:master with
~pappacena/launchpad:comment-editing-ui-bugcomment as a prerequisite.
Commit message:
Adding editing option to code review comments
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/402850
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~pappacena/launchpad:comment-editing-ui-codereview into launchpad:master.
diff --git a/lib/lp/code/browser/codereviewcomment.py b/lib/lp/code/browser/codereviewcomment.py
index 1892eb8..9dd89c2 100644
--- a/lib/lp/code/browser/codereviewcomment.py
+++ b/lib/lp/code/browser/codereviewcomment.py
@@ -59,6 +59,7 @@ from lp.services.webapp import (
Navigation,
stepthrough,
)
+from lp.services.webapp.authorization import check_permission
from lp.services.webapp.interfaces import ILaunchBag
@@ -211,6 +212,10 @@ class CodeReviewCommentView(LaunchpadView):
return download_body(
CodeReviewDisplayComment(self.context), self.request)
+@property
+def can_edit(self):
+return check_permission('launchpad.Edit', self.context.message)
+
# Should the comment be shown in full?
full_comment = True
# Show comment expanders?
diff --git a/lib/lp/code/browser/tests/test_branchmergeproposal.py b/lib/lp/code/browser/tests/test_branchmergeproposal.py
index f32350e..ba2ee85 100644
--- a/lib/lp/code/browser/tests/test_branchmergeproposal.py
+++ b/lib/lp/code/browser/tests/test_branchmergeproposal.py
@@ -2315,7 +2315,9 @@ class TestBranchMergeProposal(BrowserTestCase):
comment = self.factory.makeCodeReviewComment(body='x y' * 2000)
has_read_more = self.has_read_more(comment)
browser = self.getViewBrowser(comment.branch_merge_proposal)
-self.assertNotIn('x y' * 2000, browser.contents)
+txt = extract_text(
+find_tags_by_class(browser.contents, "comment-text")[0])
+self.assertNotIn('x y' * 2000, txt)
self.assertThat(browser.contents, has_read_more)
def test_short_conversation_comments_no_download(self):
diff --git a/lib/lp/code/stories/branches/xx-code-review-comments.txt b/lib/lp/code/stories/branches/xx-code-review-comments.txt
index 1c3738d..30d3a26 100644
--- a/lib/lp/code/stories/branches/xx-code-review-comments.txt
+++ b/lib/lp/code/stories/branches/xx-code-review-comments.txt
@@ -39,7 +39,7 @@ to the main merge proposal page.
>>> print_comments('boardCommentDetails')
Eric the Viking (eric) wrote ...
->>> print_comments('boardCommentBody')
+>>> print_comments('comment-text')
This is a very long comment about what things should be done to the
source branch to land it. When this comment is replied to, it should
wrap the line properly.
@@ -82,7 +82,7 @@ since it failed spuriously in buildbot.
After this, we are taken to the main page for the merge proposal
->>> print_comments('boardCommentBody', eric_browser, index=1)
+>>> print_comments('comment-text', eric_browser, index=1)
I like this.
I wish I had time to review it properly
This is a longer message with several lines
@@ -91,7 +91,7 @@ After this, we are taken to the main page for the merge proposal
Email addresses in code review comments are hidden for users not logged in.
>>> anon_browser.open(merge_proposal_url)
->>> print_comments('boardCommentBody', anon_browser, index=1)
+>>> print_comments('comment-text', anon_browser, index=1)
I like this.
I wish I had time to review it properly
This is a longer message with several lines
@@ -106,7 +106,7 @@ are also displayed in the new proposal.
>>> new_merge_proposal_url = canonical_url(new_merge_proposal)
>>> logout()
>>> anon_browser.open(new_merge_proposal_url)
->>> print_comments('boardCommentBody', anon_browser, index=0)
+>>> print_comments('comment-text', anon_browser, index=0)
This is a very long comment about what things should be done to the
source branch to land it. When this comment is replied to, it should
wrap the line properly.
diff --git a/lib/lp/code/templates/branchmergeproposal-index.pt b/lib/lp/code/templates/branchmergeproposal-index.pt
index 6ba4621..4a01ec5 100644
--- a/lib/lp/code/templates/branchmergeproposal-index.pt
+++ b/lib/lp/code/templates/branchmergeproposal-index.pt
@@ -244,7 +244,8 @@
'lp.code.branchmergeproposal.status', 'lp.app.comment',
'lp.app.widgets.expander',
'lp.code.branch.revisionexpander',
- 'lp.code.branchmergeproposal.inlinecomments', function(Y) {
+ 'lp.code.branchmergeproposal.inlinecomments',
+ 'lp.services.message
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-model into launchpad:master
Addressed all the comments.
Diff comments:
> diff --git a/database/schema/security.cfg b/database/schema/security.cfg
> index c3aba65..b712521 100644
> --- a/database/schema/security.cfg
> +++ b/database/schema/security.cfg
> @@ -232,7 +232,9 @@ public.logintoken = SELECT, INSERT,
> UPDATE, DELETE
> public.mailinglist = SELECT, INSERT, UPDATE, DELETE
> public.mailinglistsubscription = SELECT, INSERT, UPDATE, DELETE
> public.messageapproval = SELECT, INSERT, UPDATE, DELETE
> -public.messagechunk = SELECT, INSERT
> +public.messagechunk = SELECT, INSERT, DELETE
> +public.messagerevision = SELECT, INSERT, UPDATE, DELETE
> +public.messagerevisionchunk = SELECT, INSERT, UPDATE, DELETE
You are right. Fixing it.
> public.milestone= SELECT, INSERT, UPDATE, DELETE
> public.milestonetag = SELECT, INSERT, UPDATE, DELETE
> public.mirrorcdimagedistroseries= SELECT, INSERT, DELETE
> diff --git a/lib/lp/security.py b/lib/lp/security.py
> index 412f497..cd88d5c 100644
> --- a/lib/lp/security.py
> +++ b/lib/lp/security.py
> @@ -3182,6 +3182,15 @@ class SetMessageVisibility(AuthorizationBase):
> return (user.in_admin or user.in_registry_experts)
>
>
> +class EditMessage(AuthorizationBase):
> +permission = 'launchpad.Edit'
> +usedfor = IMessage
> +
> +def checkAuthenticated(self, user):
> +"""Only message owner can edit it."""
> +return user.isOwner(self.obj)
Right. Adding it.
> +
> +
> class ViewPublisherConfig(AdminByAdminsTeam):
> usedfor = IPublisherConfig
>
> diff --git a/lib/lp/services/messages/interfaces/messagerevision.py
> b/lib/lp/services/messages/interfaces/messagerevision.py
> new file mode 100644
> index 000..3b72673
> --- /dev/null
> +++ b/lib/lp/services/messages/interfaces/messagerevision.py
> @@ -0,0 +1,69 @@
> +# Copyright 2019-2021 Canonical Ltd. This software is licensed under the
> +# GNU Affero General Public License version 3 (see the file LICENSE).
> +
> +"""Message revision history."""
> +
> +from __future__ import absolute_import, print_function, unicode_literals
> +
> +__all__ = [
> +'IMessageRevision',
> +'IMessageRevisionChunk',
> +]
> +
> +from lazr.restful.fields import Reference
> +from zope.interface import (
> +Attribute,
> +Interface,
> +)
> +from zope.schema import (
> +Datetime,
> +Int,
> +Text,
> +)
> +
> +from lp import _
> +from lp.services.messages.interfaces.message import IMessage
> +
> +
> +class IMessageRevisionView(Interface):
> +"""IMessageRevision readable attributes."""
> +id = Int(title=_("ID"), required=True, readonly=True)
> +
> +revision = Int(title=_("Revision number"), required=True, readonly=True)
> +
> +content = Text(
> +title=_("The message at the given revision"),
> +required=False, readonly=True)
Ok!
> +
> +message = Reference(
> +title=_('The current message of this revision.'),
> +schema=IMessage, required=True, readonly=True)
> +
> +date_created = Datetime(
> +title=_("The time when this message revision was created."),
> +required=True, readonly=True)
> +
> +date_deleted = Datetime(
> +title=_("The time when this message revision was created."),
> +required=False, readonly=True)
> +
> +chunks = Attribute(_('Message pieces'))
> +
> +
> +class IMessageRevisionEdit(Interface):
> +"""IMessageRevision editable attributes."""
> +
> +def deleteContent():
> +"""Logically deletes this MessageRevision."""
Fixing this description.
> +
> +
> +class IMessageRevision(IMessageRevisionView, IMessageRevisionEdit):
> +"""A historical revision of a IMessage."""
> +
> +
> +class IMessageRevisionChunk(Interface):
> +id = Int(title=_('ID'), required=True, readonly=True)
> +messagerevision = Int(
> +title=_('MessageRevision'), required=True, readonly=True)
> +sequence = Int(title=_('Sequence order'), required=True, readonly=True)
> +content = Text(title=_('Text content'), required=False, readonly=True)
Ok!
> diff --git a/lib/lp/services/messages/model/message.py
> b/lib/lp/services/messages/model/message.py
> index e592daa..b73e212 100644
> --- a/lib/lp/services/messages/model/message.py
> +++ b/lib/lp/services/messages/model/message.py
> @@ -164,6 +176,63 @@ class Message(SQLBase):
> """See `IMessage`."""
> return None
>
> +@cachedproperty
> +def revisions(self):
> +"""See `IMessage`."""
> +return list(Store.of(self).find(
> +MessageRevision,
> +MessageRevision.message == self
> +).order_by(Desc(MessageRevision.revision)))
> +
> +def editContent(self, new_content):
> +"""See `IMessage`."""
> +store = Store.of(self)
> +
> +# Move the old
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-ui-bugcomment into launchpad:master
Thiago F. Pappacena has proposed merging
~pappacena/launchpad:comment-editing-ui-bugcomment into launchpad:master with
~pappacena/launchpad:comment-editing-ui as a prerequisite.
Commit message:
Adding editing option to bug comments
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/402604
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~pappacena/launchpad:comment-editing-ui-bugcomment into launchpad:master.
diff --git a/lib/lp/answers/stories/question-workflow.txt b/lib/lp/answers/stories/question-workflow.txt
index 7d23e23..5e39acd 100755
--- a/lib/lp/answers/stories/question-workflow.txt
+++ b/lib/lp/answers/stories/question-workflow.txt
@@ -219,7 +219,7 @@ The confirmed answer is also highlighted.
>>> print(soup.find(
-... 'div', 'boardCommentBody highlighted editable-message-body'
+... 'div', 'boardCommentBody highlighted editable-message-text'
... ).decode_contents())
New version of the firefox package are available with SVG support
enabled. You can use apt-get or adept to upgrade.
@@ -290,8 +290,8 @@ answerer back to None.
>>> bestAnswer.find('strong') is None
True
->>> bestAnswer.find('div', 'boardCommentBody editable-message-body')
->> bestAnswer.find('div', 'boardCommentBody editable-message-text')
+New version of the firefox package
are available with SVG support enabled. You can use apt-get or adept to
upgrade.
@@ -357,9 +357,9 @@ The answer's message is also highlighted as the best answer.
No Privileges Person (no-priv)
>>> message = soup.find(
-... 'div', 'boardCommentBody highlighted editable-message-body')
+... 'div', 'boardCommentBody highlighted editable-message-text')
>>> print(message)
-New version of the firefox package are
available with SVG support enabled. You can use apt-get or adept to
upgrade.
diff --git a/lib/lp/answers/templates/questionmessage-display.pt b/lib/lp/answers/templates/questionmessage-display.pt
index c4cc488..2a5be75 100644
--- a/lib/lp/answers/templates/questionmessage-display.pt
+++ b/lib/lp/answers/templates/questionmessage-display.pt
@@ -27,13 +27,13 @@
tal:attributes="title context/datecreated/fmt:datetime;
datetime context/datecreated/fmt:isodate"
tal:content="context/datecreated/fmt:displaydate">Thursday
-13:21
+13:21:
(last edit )
+ tal:content="context/date_last_edited/fmt:displaydate" />):
@@ -47,12 +47,14 @@
-
-Message text.
+
+
+ Message text.
+
diff --git a/lib/lp/bugs/browser/bugcomment.py b/lib/lp/bugs/browser/bugcomment.py
index 2302d83..2d1822e 100644
--- a/lib/lp/bugs/browser/bugcomment.py
+++ b/lib/lp/bugs/browser/bugcomment.py
@@ -52,6 +52,7 @@ from lp.services.webapp import (
Navigation,
stepthrough,
)
+from lp.services.webapp.authorization import check_permission
from lp.services.webapp.breadcrumb import Breadcrumb
from lp.services.webapp.interfaces import ILaunchBag
@@ -350,12 +351,20 @@ class BugCommentBoxView(LaunchpadView, BugCommentBoxViewMixin):
expand_reply_box = False
+@property
+def can_edit(self):
+return check_permission('launchpad.Edit', self.context)
+
class BugCommentBoxExpandedReplyView(LaunchpadView, BugCommentBoxViewMixin):
"""Render a comment box with reply field expanded."""
expand_reply_box = True
+@property
+def can_edit(self):
+return False
+
@adapter(IBugComment, IWebServiceClientRequest)
@implementer(Interface)
diff --git a/lib/lp/bugs/stories/bugattachments/xx-delete-bug-attachment.txt b/lib/lp/bugs/stories/bugattachments/xx-delete-bug-attachment.txt
index f705c8a..b9f7292 100644
--- a/lib/lp/bugs/stories/bugattachments/xx-delete-bug-attachment.txt
+++ b/lib/lp/bugs/stories/bugattachments/xx-delete-bug-attachment.txt
@@ -24,11 +24,17 @@ The attachment is now visible on the bug page.
There will also be a comment with a link to the attachment in its body.
>>> print_comments(user_browser.contents)
-This would be a real killer feature...
+This would be a real killer
+ feature...
-Oddly enough the bug system seems only capable...
+Oddly enough the bug system seems
+ only capable...
Attachment: Great deal
+
@@ -80,9 +86,13 @@ Since the attachment has been deleted, the comment referencing it will no
longer be visible.
>>> print_comments(user_browser.contents)
-This would be a real killer feature...
+This woul
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-db-patch into launchpad:db-devel
Diff comments: > diff --git a/database/schema/patch-2210-31-0.sql > b/database/schema/patch-2210-31-0.sql > new file mode 100644 > index 000..593d214 > --- /dev/null > +++ b/database/schema/patch-2210-31-0.sql > @@ -0,0 +1,28 @@ > +-- Copyright 2021 Canonical Ltd. This software is licensed under the > +-- GNU Affero General Public License version 3 (see the file LICENSE). > + > +SET client_min_messages=ERROR; > + > +ALTER TABLE Message > +ADD COLUMN date_deleted timestamp without time zone, > +ADD COLUMN date_last_edited timestamp without time zone; > + > +CREATE TABLE MessageRevision ( > +id serial PRIMARY KEY, > +message integer NOT NULL REFERENCES Message, > +date_created timestamp without time zone, Right! > +date_deleted timestamp without time zone > +); It makes sense. I'll add the column here, so we don't need to change this table's structure once we implement subject editing too. > + > +CREATE UNIQUE INDEX messagerevision__message__date_created__key > +ON MessageRevision(message, date_created); Sure. > + > +CREATE TABLE MessageRevisionChunk ( > +id serial PRIMARY KEY, > +messagerevision integer NOT NULL REFERENCES MessageRevisionChunk, Yep... wrong copy+paste mistake. Fixing it. > +sequence integer NOT NULL, > +content text NOT NULL > +); Even with the new editing feature, I expect that most Messages and MessageRevisions will never be updated (and even those ones updated will not change frequently, so VACUUM would take care of removing old tuples and keeping things in the same block). fillfactor=100 seems to be a reasonable choice in these cases too. According to the docs, fillfactor=100 seems to be the default, but I'll add it explicitly. > + > + > +INSERT INTO LaunchpadDatabaseRevision VALUES (2210, 31, 0); -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401976 Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-db-patch. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-db-patch into launchpad:db-devel
Pushed the requested changes. It worth another review round. Diff comments: > diff --git a/database/schema/patch-2210-31-0.sql > b/database/schema/patch-2210-31-0.sql > new file mode 100644 > index 000..9bcceb5 > --- /dev/null > +++ b/database/schema/patch-2210-31-0.sql > @@ -0,0 +1,21 @@ > +-- Copyright 2021 Canonical Ltd. This software is licensed under the > +-- GNU Affero General Public License version 3 (see the file LICENSE). > + > +SET client_min_messages=ERROR; > + > +ALTER TABLE Message > +ADD COLUMN date_deleted timestamp without time zone, > +ADD COLUMN date_last_edit timestamp without time zone; Fixing it! > + > +CREATE TABLE MessageRevision ( > +id serial PRIMARY KEY, > +message integer NOT NULL REFERENCES Message, > +content text, `Message.text_contents` fetches the chunks, and I was a bit worried that increasing query time there could affect load time of the UI. Anyway, I'm pushing the changes here and in following MPs to have a MessageRevisionChunk table and implement the above steps when editing a message. > +date_created timestamp without time zone, > +date_deleted timestamp without time zone > +); > + > +CREATE UNIQUE INDEX messagerevision__message__date_created__key > +ON MessageRevision(message, date_created); > + > +INSERT INTO LaunchpadDatabaseRevision VALUES (2210, 31, 0); -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401976 Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-db-patch. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-ui into launchpad:master
Thiago F. Pappacena has proposed merging
~pappacena/launchpad:comment-editing-ui into launchpad:master with
~pappacena/launchpad:comment-editing-revisions-api as a prerequisite.
Commit message:
Javascript component to edit messages, and its first usage in QuestionMessage
view
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/402522
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~pappacena/launchpad:comment-editing-ui into launchpad:master.
diff --git a/lib/canonical/launchpad/icing/css/base.scss b/lib/canonical/launchpad/icing/css/base.scss
index 68bcce1..6ee2779 100644
--- a/lib/canonical/launchpad/icing/css/base.scss
+++ b/lib/canonical/launchpad/icing/css/base.scss
@@ -2,7 +2,8 @@
body {
/* line-height is the same as the sprite height. */
-font-family: Ubuntu, 'Bitstream Vera Sans', 'DejaVu Sans', Tahoma, sans-serif;
+font-family: Ubuntu, 'Bitstream Vera Sans', 'DejaVu Sans', Tahoma,
+ sans-serif;
font-size: 12px;
line-height: 18px;
color: #333;
@@ -444,7 +445,8 @@ body {
table {
th, td {
- /* We don't want extra padding on nested tables, like batch navigation. */
+ /* We don't want extra padding on nested tables,
+ like batch navigation. */
padding: 0;
}
}
@@ -543,6 +545,42 @@ body {
border-bottom-left-radius: 5px;
}
+ .editable-message {
+.editable-message-notification {
+ position: absolute;
+ width: 100%;
+ height: 100%;
+ top: 0;
+ left: 0;
+ background-color: white;
+ opacity: 0.9;
+ display: flex;
+ flex-wrap: wrap;
+ justify-content: center;
+ align-items: center;
+
+ p {
+display: block;
+flex-basis: 100%;
+margin-top: 10px;
+ }
+ .editable-message-notification-dismiss {
+flex-basis: 100%;
+text-align: center;
+padding: 1px;
+margin-top: -10px;
+ }
+}
+
+.editable-message-form {
+ padding: 0.5em 12px 0;
+ input[type="button"] {
+padding: 1px;
+margin: 5px;
+ }
+}
+ }
+
@import 'typography',
'colours',
'forms',
diff --git a/lib/lp/answers/browser/question.py b/lib/lp/answers/browser/question.py
index 18b4c47..104017e 100644
--- a/lib/lp/answers/browser/question.py
+++ b/lib/lp/answers/browser/question.py
@@ -1191,8 +1191,14 @@ class QuestionMessageDisplayView(LaunchpadView):
# If a comment that isn't visible is being rendered, it's being
# rendered for an admin or registry_expert.
css_classes.append("adminHiddenComment")
+if self.can_edit:
+css_classes.append("editable-message")
return " ".join(css_classes)
+@property
+def can_edit(self):
+return check_permission('launchpad.Edit', self.context)
+
def canConfirmAnswer(self):
"""Return True if the user can confirm this answer."""
return (self.display_confirm_button and
diff --git a/lib/lp/answers/stories/question-workflow.txt b/lib/lp/answers/stories/question-workflow.txt
index 984aaf4..7d23e23 100755
--- a/lib/lp/answers/stories/question-workflow.txt
+++ b/lib/lp/answers/stories/question-workflow.txt
@@ -219,7 +219,8 @@ The confirmed answer is also highlighted.
>>> print(soup.find(
-... 'div', 'boardCommentBody highlighted').decode_contents())
+... 'div', 'boardCommentBody highlighted editable-message-body'
+... ).decode_contents())
New version of the firefox package are available with SVG support
enabled. You can use apt-get or adept to upgrade.
@@ -289,9 +290,9 @@ answerer back to None.
>>> bestAnswer.find('strong') is None
True
->>> bestAnswer.find('div', 'boardCommentBody')
-New version
-of the firefox package
+>>> bestAnswer.find('div', 'boardCommentBody editable-message-body')
+New version of the firefox package
are available with SVG support enabled. You can use apt-get or adept to
upgrade.
@@ -356,9 +357,9 @@ The answer's message is also highlighted as the best answer.
No Privileges Person (no-priv)
>>> message = soup.find(
-... 'div', 'boardCommentBody highlighted')
+... 'div', 'boardCommentBody highlighted editable-message-body')
>>> print(message)
-New version of the firefox package are
available with SVG support enabled. You can use apt-get or adept to
upgrade.
diff --git a/lib/lp/answers/templates/question-index.pt b/lib/lp/answers/templates/question-index.pt
index e7e233d..13f6270 100644
--- a/lib/lp/answers/templates/question-index.pt
+++ b/lib/lp/answers/templates/question-index.pt
@@ -17,
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-db-patch into launchpad:db-devel
Diff comments: > diff --git a/database/schema/patch-2210-31-0.sql > b/database/schema/patch-2210-31-0.sql > new file mode 100644 > index 000..9bcceb5 > --- /dev/null > +++ b/database/schema/patch-2210-31-0.sql > @@ -0,0 +1,21 @@ > +-- Copyright 2021 Canonical Ltd. This software is licensed under the > +-- GNU Affero General Public License version 3 (see the file LICENSE). > + > +SET client_min_messages=ERROR; > + > +ALTER TABLE Message > +ADD COLUMN date_deleted timestamp without time zone, > +ADD COLUMN date_last_edit timestamp without time zone; > + > +CREATE TABLE MessageRevision ( > +id serial PRIMARY KEY, > +message integer NOT NULL REFERENCES Message, > +content text, Indeed, this is not covering well multi-part messages and losing information in such cases, specially the `blob` of those chunks. But I think I don't fully agree on putting all historical revisions directly into `Message` / `MessageChunk`. By doing so, we would increase complexity on reading queries in several places, and those datasets would start growing faster than they are doing today, which could potentially make querying them even more expensive in a near-ish future. I would suggest another approach: keeping `Message` and `MessageChunk` tables as "the current state of the messages and their chunks", as they are today. But instead of having just `MessageRevision` with the historical text, mimic the `MessageChunk` structure in a `MessageRevisionChunk` table. This way, when a user edits the text part of a message, we would: - create a new `MessageRevision`; - move all existing message chunks with `content` to the new `MessageRevisionChunk` table, linked to the newly created `MessageRevision`; - remove all current `MessageChunk` with `content` (keeping the ones with `blobs`); - create a single `MessageChunk` with the new text `content`. Do you think this covers all our cases without losing anything? > +date_created timestamp without time zone, > +date_deleted timestamp without time zone > +); > + > +CREATE UNIQUE INDEX messagerevision__message__date_created__key > +ON MessageRevision(message, date_created); > + > +INSERT INTO LaunchpadDatabaseRevision VALUES (2210, 31, 0); -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401976 Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:comment-editing-db-patch. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-revisions-api into launchpad:master
Thiago F. Pappacena has proposed merging
~pappacena/launchpad:comment-editing-revisions-api into launchpad:master with
~pappacena/launchpad:comment-editing-api as a prerequisite.
Commit message:
API to get and delete comment's revision history for bug messages, answers and
code review comments
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/402285
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~pappacena/launchpad:comment-editing-revisions-api into launchpad:master.
diff --git a/lib/lp/_schema_circular_imports.py b/lib/lp/_schema_circular_imports.py
index a61d39d..4b1ecff 100644
--- a/lib/lp/_schema_circular_imports.py
+++ b/lib/lp/_schema_circular_imports.py
@@ -149,6 +149,7 @@ from lp.services.messages.interfaces.message import (
IMessage,
IUserToUserEmail,
)
+from lp.services.messages.interfaces.messagerevision import IMessageRevision
from lp.services.webservice.apihelpers import (
patch_collection_property,
patch_collection_return_type,
@@ -612,6 +613,7 @@ patch_reference_property(IIndexedMessage, 'inside', IBugTask)
# IMessage
patch_reference_property(IMessage, 'owner', IPerson)
+patch_collection_property(IMessage, 'revisions', IMessageRevision)
# IUserToUserEmail
patch_reference_property(IUserToUserEmail, 'sender', IPerson)
diff --git a/lib/lp/answers/browser/configure.zcml b/lib/lp/answers/browser/configure.zcml
index 631e8b8..87a6b76 100644
--- a/lib/lp/answers/browser/configure.zcml
+++ b/lib/lp/answers/browser/configure.zcml
@@ -282,6 +282,10 @@
module=".question"
classes="QuestionNavigation"
/>
+
http://api.launchpad.test/devel/my-project/+question/...'
resource_type_link: 'http://api.launchpad.test/devel/#question_message'
+revisions_collection_link: 'http://...'
self_link:
'http://api.launchpad.test/devel/my-project/+question/.../messages/1'
subject: 'Re: Q 1 great'
diff --git a/lib/lp/bugs/browser/bugcomment.py b/lib/lp/bugs/browser/bugcomment.py
index e40967f..2302d83 100644
--- a/lib/lp/bugs/browser/bugcomment.py
+++ b/lib/lp/bugs/browser/bugcomment.py
@@ -49,6 +49,8 @@ from lp.services.propertycache import (
from lp.services.webapp import (
canonical_url,
LaunchpadView,
+Navigation,
+stepthrough,
)
from lp.services.webapp.breadcrumb import Breadcrumb
from lp.services.webapp.interfaces import ILaunchBag
@@ -57,6 +59,16 @@ 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_comments(self, index):
+index = int(index) - 1
+return self.context.revisions[index]
+
+
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/bugs/browser/configure.zcml b/lib/lp/bugs/browser/configure.zcml
index ac80819..d9c36a8 100644
--- a/lib/lp/bugs/browser/configure.zcml
+++ b/lib/lp/bugs/browser/configure.zcml
@@ -1,4 +1,4 @@
-
@@ -163,6 +163,10 @@
path_expression="string:comments/${index}"
attribute_to_parent="bugtask"
rootsite="bugs"/>
+
http://.../~name12'
parent_link: None
resource_type_link: 'http://.../#message'
+revisions_collection_link: '...'
self_link: 'http://.../firefox/+bug/5/comments/0'
subject: 'Firefox install instructions should be complete'
web_link: 'http://bugs.../firefox/+bug/5/comments/0'
@@ -284,6 +285,7 @@ We can add a new message to a bug by calling the newMessage method.
content: 'This is a new message added through the webservice API.'
...
resource_type_link: 'http://api.launchpad.test/beta/#message'
+revisions_collection_link: '...'
self_link: 'http://api.launchpad.test/beta/firefox/+bug/5/comments/1'
subject: 'A new message'
web_link: '...'
@@ -1354,6 +1356,7 @@ attachment. This is where our comment is recorded.
owner_link: 'http://.../~salgado'
parent_link: None
resource_type_link: 'http://.../#message'
+revisions_collection_link: '...'
self_link: 'http://.../firefox/+bug/1/comments/2'
subject: 'Re: Firefox does not support SVG'
web_link: 'http://bugs.../firefox/+bug/1/comments/2'
diff --git a/lib/lp/code/browser/codereviewcomment.py b/lib/lp/code/browser/codereviewcomment.py
index 0d0496d..c4ca667 100644
--- a/lib/lp/code/browser/codereviewcomment.py
+++ b/lib/lp/code/browser/codereviewcomment.py
@@ -56,10 +56,22 @@ from lp.services.webapp import (
ContextMenu,
LaunchpadView,
Link,
+Navigation,
+
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-model into launchpad:master
The proposal to merge ~pappacena/launchpad:comment-editing-model into launchpad:master has been updated. Description changed to: More changes will be added in a future MP in order to adjust BugComment, CodeReviewComment and QuestionMessage to use the new methods. These changes were splitted to make the review process easier. For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401894 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:comment-editing-model into launchpad:master. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-model into launchpad:master
The proposal to merge ~pappacena/launchpad:comment-editing-model into launchpad:master has been updated. Description changed to: More changes will be added in a future MP in order to adjust BugComment, CodeReviewComment and QuestionMessage to use the new methods. Those changes were splitted to make the review process easier. For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401894 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:comment-editing-model into launchpad:master. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-api into launchpad:master
Thiago F. Pappacena has proposed merging
~pappacena/launchpad:comment-editing-api into launchpad:master with
~pappacena/launchpad:comment-editing-model as a prerequisite.
Commit message:
API to edit and delete comments for bug messages, answers and code review
comments
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/402211
The API for the message revision history (listing and deleting old revisions)
will be done in a future MP, in order to keep the review shorter.
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~pappacena/launchpad:comment-editing-api into launchpad:master.
diff --git a/lib/lp/answers/browser/configure.zcml b/lib/lp/answers/browser/configure.zcml
index 46c7553..631e8b8 100644
--- a/lib/lp/answers/browser/configure.zcml
+++ b/lib/lp/answers/browser/configure.zcml
@@ -1,4 +1,4 @@
-
@@ -278,6 +278,10 @@
module=".question"
classes="QuestionSetNavigation"
/>
+
@@ -142,7 +142,9 @@
-
+
+
http://api.launchpad.test/devel/~contact'
diff --git a/lib/lp/answers/tests/test_question_workflow.py b/lib/lp/answers/tests/test_question_workflow.py
index 6450d61..66fa81b 100644
--- a/lib/lp/answers/tests/test_question_workflow.py
+++ b/lib/lp/answers/tests/test_question_workflow.py
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Test the question workflow methods.
@@ -53,6 +53,7 @@ from lp.testing import (
ANONYMOUS,
login,
login_person,
+person_logged_in,
TestCase,
)
from lp.testing.fixture import ZopeEventHandlerFixture
@@ -238,7 +239,8 @@ class BaseAnswerTrackerWorkflowTestCase(TestCase):
It also verifies that the question status, datelastquery (or
datelastresponse) were updated to reflect the time of the message.
"""
-self.assertTrue(verifyObject(IQuestionMessage, message))
+with person_logged_in(message.owner):
+self.assertTrue(verifyObject(IQuestionMessage, message))
self.assertEqual("Re: Help!", message.subject)
self.assertEqual(expected_owner, message.owner)
diff --git a/lib/lp/bugs/browser/bugcomment.py b/lib/lp/bugs/browser/bugcomment.py
index 2b61552..e40967f 100644
--- a/lib/lp/bugs/browser/bugcomment.py
+++ b/lib/lp/bugs/browser/bugcomment.py
@@ -1,4 +1,4 @@
-# Copyright 2006-2020 Canonical Ltd. This software is licensed under the
+# Copyright 2006-2021 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Bug comment browser view classes."""
@@ -72,7 +72,10 @@ def build_comments_from_chunks(
cache = get_property_cache(message)
if getattr(cache, 'chunks', None) is None:
cache.chunks = []
-cache.chunks.append(removeSecurityProxy(chunk))
+# Soft-deleted messages will have None chunk here. Skip cache
+# filling in this case.
+if chunk is not None:
+cache.chunks.append(removeSecurityProxy(chunk))
bug_comment = comments.get(message.id)
if bug_comment is None:
if bugmessage.index == 0 and hide_first:
diff --git a/lib/lp/bugs/configure.zcml b/lib/lp/bugs/configure.zcml
index 73b99e8..b2a54ac 100644
--- a/lib/lp/bugs/configure.zcml
+++ b/lib/lp/bugs/configure.zcml
@@ -1,4 +1,4 @@
-
@@ -773,7 +773,9 @@
+interface="lp.bugs.interfaces.bugmessage.IBugMessageView"/>
+
diff --git a/lib/lp/bugs/interfaces/bugmessage.py b/lib/lp/bugs/interfaces/bugmessage.py
index 98c18f3..15b3373 100644
--- a/lib/lp/bugs/interfaces/bugmessage.py
+++ b/lib/lp/bugs/interfaces/bugmessage.py
@@ -1,4 +1,4 @@
-# Copyright 2004-2020 Canonical Ltd. This software is licensed under the
+# Copyright 2004-2021 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Bug message interfaces."""
@@ -31,11 +31,14 @@ from lp.bugs.interfaces.hasbug import IHasBug
from lp.registry.interfaces.person import IPerson
from lp.services.comments.interfaces.conversation import IComment
from lp.services.fields import Title
-from lp.services.messages.interfaces.message import IMessage
+from lp.services.messages.interfaces.message import (
+IMessage,
+IMessageView,
+)
-class IBugMessage(IHasBug):
-"""A link between a bug and a message."""
+class IBugMessageView(IMessageView, IHasBug):
+"""Public attributes for a link between a bug and a messa
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:bug-oci-project-navigation into launchpad:master
The proposal to merge ~pappacena/launchpad:bug-oci-project-navigation into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/394181 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:bugtask-oci-project. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:bug-oci-project-navigation into launchpad:master
The proposal to merge ~pappacena/launchpad:bug-oci-project-navigation into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/394181 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:bugtask-oci-project. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-model into launchpad:master
Thiago F. Pappacena has proposed merging
~pappacena/launchpad:comment-editing-model into launchpad:master.
Commit message:
Mapping database initial structure for message editing
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401894
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~pappacena/launchpad:comment-editing-model into launchpad:master.
diff --git a/database/schema/security.cfg b/database/schema/security.cfg
index 1bbb845..c8b7d10 100644
--- a/database/schema/security.cfg
+++ b/database/schema/security.cfg
@@ -232,7 +232,8 @@ public.logintoken = SELECT, INSERT, UPDATE, DELETE
public.mailinglist = SELECT, INSERT, UPDATE, DELETE
public.mailinglistsubscription = SELECT, INSERT, UPDATE, DELETE
public.messageapproval = SELECT, INSERT, UPDATE, DELETE
-public.messagechunk = SELECT, INSERT
+public.messagechunk = SELECT, INSERT, DELETE
+public.messagerevision = SELECT, INSERT, UPDATE
public.milestone= SELECT, INSERT, UPDATE, DELETE
public.milestonetag = SELECT, INSERT, UPDATE, DELETE
public.mirrorcdimagedistroseries= SELECT, INSERT, DELETE
diff --git a/lib/lp/bugs/browser/tests/test_bugcomment.py b/lib/lp/bugs/browser/tests/test_bugcomment.py
index c1877a5..e5baac1 100644
--- a/lib/lp/bugs/browser/tests/test_bugcomment.py
+++ b/lib/lp/bugs/browser/tests/test_bugcomment.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2010-2021 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Tests for the bugcomment module."""
@@ -35,6 +35,7 @@ from lp.testing import (
BrowserTestCase,
celebrity_logged_in,
login_person,
+person_logged_in,
TestCase,
TestCaseWithFactory,
verifyObject,
@@ -300,7 +301,10 @@ class TestBugCommentImplementsInterface(TestCaseWithFactory):
bug_message = self.factory.makeBugComment()
bugtask = bug_message.bugs[0].bugtasks[0]
bug_comment = BugComment(1, bug_message, bugtask)
-verifyObject(IBugComment, bug_comment)
+# Runs verifyObject logged in as the bug owner, so we don't fail on
+# attributes that are not public to everyone.
+with person_logged_in(bug_message.owner):
+verifyObject(IBugComment, bug_comment)
def test_download_url(self):
"""download_url is provided and works as expected."""
diff --git a/lib/lp/security.py b/lib/lp/security.py
index 412f497..cd88d5c 100644
--- a/lib/lp/security.py
+++ b/lib/lp/security.py
@@ -3182,6 +3182,15 @@ class SetMessageVisibility(AuthorizationBase):
return (user.in_admin or user.in_registry_experts)
+class EditMessage(AuthorizationBase):
+permission = 'launchpad.Edit'
+usedfor = IMessage
+
+def checkAuthenticated(self, user):
+"""Only message owner can edit it."""
+return user.isOwner(self.obj)
+
+
class ViewPublisherConfig(AdminByAdminsTeam):
usedfor = IPublisherConfig
diff --git a/lib/lp/services/messages/configure.zcml b/lib/lp/services/messages/configure.zcml
index e989fe2..fcaffab 100644
--- a/lib/lp/services/messages/configure.zcml
+++ b/lib/lp/services/messages/configure.zcml
@@ -1,4 +1,4 @@
-
@@ -10,9 +10,9 @@
i18n_domain="launchpad">
-
-
+
+
@@ -22,6 +22,19 @@
+
+
+
+
+
+
+
diff --git a/lib/lp/services/messages/interfaces/message.py b/lib/lp/services/messages/interfaces/message.py
index 7e18e7d..0c54c84 100644
--- a/lib/lp/services/messages/interfaces/message.py
+++ b/lib/lp/services/messages/interfaces/message.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -49,9 +49,19 @@ from lp.services.librarian.interfaces import ILibraryFileAlias
from lp.services.webservice.apihelpers import patch_reference_property
-@exported_as_webservice_entry('message')
-class IMessage(Interface):
-"""A message.
+class IMessageEdit(Interface):
+
+def edit_content(new_content):
+"""Edit the content of this message, generating a new message
+revision with the old content.
+"""
+
+def delete_content():
+"""Deletes this message content."""
+
+
+class IMessageView(Interface):
+&qu
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:comment-editing-db-patch into launchpad:db-devel
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:comment-editing-db-patch into launchpad:db-devel. Commit message: Database patch for comment editing feature Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401976 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:comment-editing-db-patch into launchpad:db-devel. diff --git a/database/schema/patch-2210-31-0.sql b/database/schema/patch-2210-31-0.sql new file mode 100644 index 000..9bcceb5 --- /dev/null +++ b/database/schema/patch-2210-31-0.sql @@ -0,0 +1,21 @@ +-- Copyright 2021 Canonical Ltd. This software is licensed under the +-- GNU Affero General Public License version 3 (see the file LICENSE). + +SET client_min_messages=ERROR; + +ALTER TABLE Message +ADD COLUMN date_deleted timestamp without time zone, +ADD COLUMN date_last_edit timestamp without time zone; + +CREATE TABLE MessageRevision ( +id serial PRIMARY KEY, +message integer NOT NULL REFERENCES Message, +content text, +date_created timestamp without time zone, +date_deleted timestamp without time zone +); + +CREATE UNIQUE INDEX messagerevision__message__date_created__key +ON MessageRevision(message, date_created); + +INSERT INTO LaunchpadDatabaseRevision VALUES (2210, 31, 0); ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:delete-oci-project into launchpad:master
The proposal to merge ~pappacena/launchpad:delete-oci-project into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401424 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:delete-oci-project. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:delete-oci-project into launchpad:master
Added the requested change. It should be good to go now.
Diff comments:
> diff --git a/lib/lp/registry/model/ociproject.py
> b/lib/lp/registry/model/ociproject.py
> index 3cf675f..15f552c 100644
> --- a/lib/lp/registry/model/ociproject.py
> +++ b/lib/lp/registry/model/ociproject.py
> @@ -299,6 +300,36 @@ class OCIProject(BugTargetBase, StormBase):
> return []
> return BRANCH_POLICY_ALLOWED_TYPES[self.pillar.branch_sharing_policy]
>
> +def destroySelf(self):
> +"""See `IOCIProject`."""
> +from lp.bugs.model.bugtask import BugTask
> +from lp.code.model.gitrepository import GitRepository
> +from lp.oci.model.ocirecipe import OCIRecipe
> +
> +# Cannot delete this OCI project if it has recipes associated if it.
> +exists_recipes = not IStore(OCIRecipe).find(
> +OCIRecipe,
> +OCIRecipe.oci_project == self).is_empty()
> +if exists_recipes:
> +raise CannotDeleteOCIProject("This OCI project contains
> recipes.")
> +
> +# Cannot delete this OCI project if it has bugs associated with it.
> +exists_bugs = not IStore(BugTask).find(
> +BugTask, BugTask.ociproject == self).is_empty()
It makes sense. Adding and XXX with guidelines for when we map the
BugTask.ociprojectseries field.
> +if exists_bugs:
> +raise CannotDeleteOCIProject("This OCI project contains bugs.")
> +
> +git_repos = IStore(GitRepository).find(
> +GitRepository, GitRepository.oci_project == self)
> +if not git_repos.is_empty():
> +repos = ", ".join(repo.display_name for repo in git_repos)
> +raise CannotDeleteOCIProject(
> +"There are git repositories associated with this OCI
> project: "
> ++ repos)
> +for series in self.series:
> +series.destroySelf()
> +IStore(self).remove(self)
> +
>
> @implementer(IOCIProjectSet)
> class OCIProjectSet:
--
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401424
Your team Launchpad code reviewers is subscribed to branch
~pappacena/launchpad:delete-oci-project.
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:delete-oci-project into launchpad:master
Diff comments:
> diff --git a/lib/lp/registry/model/ociproject.py
> b/lib/lp/registry/model/ociproject.py
> index 3cf675f..15f552c 100644
> --- a/lib/lp/registry/model/ociproject.py
> +++ b/lib/lp/registry/model/ociproject.py
> @@ -299,6 +300,36 @@ class OCIProject(BugTargetBase, StormBase):
> return []
> return BRANCH_POLICY_ALLOWED_TYPES[self.pillar.branch_sharing_policy]
>
> +def destroySelf(self):
> +"""See `IOCIProject`."""
> +from lp.bugs.model.bugtask import BugTask
> +from lp.code.model.gitrepository import GitRepository
> +from lp.oci.model.ocirecipe import OCIRecipe
> +
> +# Cannot delete this OCI project if it has recipes associated if it.
> +exists_recipes = not IStore(OCIRecipe).find(
> +OCIRecipe,
> +OCIRecipe.oci_project == self).is_empty()
> +if exists_recipes:
> +raise CannotDeleteOCIProject("This OCI project contains
> recipes.")
> +
> +# Cannot delete this OCI project if it has bugs associated with it.
> +exists_bugs = not IStore(BugTask).find(
> +BugTask, BugTask.ociproject == self).is_empty()
Although we already have it prepared at database level, we are not currently
supporting bugs for OCIProjectSeries (the database column is not even mapped at
the BugTask model, since we don't have a UI to manage that today).
> +if exists_bugs:
> +raise CannotDeleteOCIProject("This OCI project contains bugs.")
> +
> +git_repos = IStore(GitRepository).find(
> +GitRepository, GitRepository.oci_project == self)
> +if not git_repos.is_empty():
> +repos = ", ".join(repo.display_name for repo in git_repos)
> +raise CannotDeleteOCIProject(
> +"There are git repositories associated with this OCI
> project: "
> ++ repos)
I agree. And we have a reasonable way of listing those git repositories in the
UI already. I'll remove this.
> +for series in self.series:
> +series.destroySelf()
> +IStore(self).remove(self)
> +
>
> @implementer(IOCIProjectSet)
> class OCIProjectSet:
--
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401424
Your team Launchpad code reviewers is subscribed to branch
~pappacena/launchpad:delete-oci-project.
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:delete-oci-project into launchpad:master
Added the check to prevent deletion when there are bugs associated with the OCI
project.
It should be good for another review round now.
Diff comments:
> diff --git a/lib/lp/registry/model/ociproject.py
> b/lib/lp/registry/model/ociproject.py
> index e98b3ea..da0bb98 100644
> --- a/lib/lp/registry/model/ociproject.py
> +++ b/lib/lp/registry/model/ociproject.py
> @@ -297,6 +298,28 @@ class OCIProject(BugTargetBase, StormBase):
> namespace = getUtility(IGitNamespaceSet).get(person,
> oci_project=self)
> return namespace.name
>
> +def destroySelf(self):
> +"""See `IOCIProject`."""
> +from lp.oci.model.ocirecipe import OCIRecipe
> +from lp.code.model.gitrepository import GitRepository
> +
> +exists_recipes = not IStore(OCIRecipe).find(
> +OCIRecipe,
> +OCIRecipe.oci_project == self).is_empty()
> +if exists_recipes:
> +raise CannotDeleteOCIProject("This OCI recipe contains recipes.")
Absolutely! Fixing it.
> +
> +git_repos = IStore(GitRepository).find(
> +GitRepository, GitRepository.oci_project == self)
> +if not git_repos.is_empty():
> +repos = ", ".join(repo.display_name for repo in git_repos)
> +raise CannotDeleteOCIProject(
> +"There are git repositories associated with this OCI
> project: "
> ++ repos)
> +for series in self.series:
> +series.destroySelf()
> +IStore(self).remove(self)
> +
>
> @implementer(IOCIProjectSet)
> class OCIProjectSet:
--
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401424
Your team Launchpad code reviewers is subscribed to branch
~pappacena/launchpad:delete-oci-project.
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:bug-oci-project-navigation into launchpad:master
Ah, right! Just rejected that MP! Thanks! -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/394181 Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:bugtask-oci-project. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:bug-oci-project-selector-ui into launchpad:master
The proposal to merge ~pappacena/launchpad:bug-oci-project-selector-ui into launchpad:master has been updated. Status: Needs review => Rejected For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/394210 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:bug-oci-project-selector-ui into launchpad:master. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:bugtask-oci-project into launchpad:master
The proposal to merge ~pappacena/launchpad:bugtask-oci-project into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/393570 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:stormify-bug-task. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:bug-oci-project-navigation into launchpad:master
The proposal to merge ~pappacena/launchpad:bug-oci-project-navigation into launchpad:master has been updated. Status: Approved => Needs review For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/394181 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:bugtask-oci-project. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:bug-oci-project-navigation into launchpad:master
The proposal to merge ~pappacena/launchpad:bug-oci-project-navigation into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/394181 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:bugtask-oci-project. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:bug-oci-project-navigation into launchpad:master
Pushed the requested changes.
About withdrawing the push hint, maybe it would make sense to move it to the
"Code" tab instead of removing it completely? The hint itself seems to be
valuable. Also, I guess we should do it in a separate MP.
Diff comments:
> diff --git a/lib/lp/app/widgets/launchpadtarget.py
> b/lib/lp/app/widgets/launchpadtarget.py
> index 00f0b37..e2fe87c 100644
> --- a/lib/lp/app/widgets/launchpadtarget.py
> +++ b/lib/lp/app/widgets/launchpadtarget.py
> @@ -55,15 +55,17 @@ class LaunchpadTargetWidget(BrowserWidget, InputWidget):
> def getProductVocabulary(self):
> return 'Product'
>
> -def setUpSubWidgets(self):
> -if self._widgets_set_up:
> -return
> +def getPackageVocabulary(self):
Ok!
> if bool(getFeatureFlag('disclosure.dsp_picker.enabled')):
> # Replace the default field with a field that uses the better
> # vocabulary.
> -package_vocab = 'DistributionSourcePackage'
> +return 'DistributionSourcePackage'
> else:
> -package_vocab = 'BinaryAndSourcePackageName'
> +return 'BinaryAndSourcePackageName'
> +
> +def setUpSubWidgets(self):
> +if self._widgets_set_up:
> +return
> fields = [
> Choice(
> __name__='product', title=u'Project',
> diff --git a/lib/lp/bugs/stories/bugtask-management/xx-bugtask-edit-forms.txt
> b/lib/lp/bugs/stories/bugtask-management/xx-bugtask-edit-forms.txt
> index 49341d1..174a261 100644
> --- a/lib/lp/bugs/stories/bugtask-management/xx-bugtask-edit-forms.txt
> +++ b/lib/lp/bugs/stories/bugtask-management/xx-bugtask-edit-forms.txt
> @@ -49,6 +49,7 @@ respective bug task.
> Target
> Distribution
> ...
> +Package (Find...)
> Project (Find…)
Not really. It works fine either way, so I'll make the new one use the same
style as the existing one.
> Status Importance Milestone
> New... Low... (nothing selected)...
> diff --git a/lib/lp/registry/browser/ociproject.py
> b/lib/lp/registry/browser/ociproject.py
> index 3f938d8..3fada35 100644
> --- a/lib/lp/registry/browser/ociproject.py
> +++ b/lib/lp/registry/browser/ociproject.py
> @@ -175,8 +178,23 @@ class OCIProjectFacets(StandardLaunchpadFacets):
> enable_only = [
> 'overview',
> 'branches',
> +'bugs',
> ]
>
> +def makeLink(self, text, context, view_name, site):
> +site = 'mainsite' if self.mainsite_only else site
> +target = canonical_url(context, view_name=view_name, rootsite=site)
> +return Link(target, text, site=site)
> +
> +def branches(self):
> +return self.makeLink('Code', self.context.pillar, '+branches',
> 'code')
Ok!
> +
> +def bugs(self):
> +"""Override bugs link to show the OCIProject's bug page, instead of
> +the pillar's bug page.
> +"""
> +return self.makeLink('Bugs', self.context, '+bugs', 'bugs')
> +
>
> class OCIProjectNavigationMenu(NavigationMenu):
> """Navigation menu for OCI projects."""
> diff --git a/lib/lp/registry/interfaces/ociproject.py
> b/lib/lp/registry/interfaces/ociproject.py
> index 85fb588..d2cabc8 100644
> --- a/lib/lp/registry/interfaces/ociproject.py
> +++ b/lib/lp/registry/interfaces/ociproject.py
> @@ -67,7 +72,9 @@ from lp.services.fields import (
> OCI_PROJECT_ALLOW_CREATE = 'oci.project.create.enabled'
>
>
> -class IOCIProjectView(IHasGitRepositories, Interface):
> +class IOCIProjectView(IHasGitRepositories, IServiceUsage,
By having its own "Overview", "Code" and "Bugs" pages, OCIProject behaves a bit
more like a Distribution or a Product itself, isn't it?
DistributionSourcePacakges would be a bit more like OCIRecipes here.
Bugs page, for example, tries to adapt OCIProject to IServiceUsage at some
point (to control the facet links at the top), and this seems to be the easier
way to make that work.
> + IHasOfficialBugTags, IStructuralSubscriptionTarget,
> + Interface):
> """IOCIProject attributes that require launchpad.View permission."""
>
> id = Int(title=_("ID"), required=True, readonly=True)
> diff --git a/lib/lp/registry/vocabularies.py b/lib/lp/registry/vocabularies.py
> index 02e3158..e59028e 100644
> --- a/lib/lp/registry/vocabularies.py
> +++ b/lib/lp/registry/vocabularies.py
> @@ -2229,5 +2240,75 @@ class OCIProjectVocabulary(StormVocabularyBase):
> def search(self, query, vocab_filter=None):
> return getUtility(IOCIProjectSet).searchByName(query)
>
> +@property
It is probably not being used at all, but I'll change it to property in this MP
anyway, and leave tests around it to be done after.
> def _entries(self):
> -return getUtility(IOCIProjectSet).searchByName('')
> +return getUtility(IOCIProjectSet).searchByName(six.ensure_text(''))
Ok!
> +
> +def __contains__(self, obj):
>
[Launchpad-reviewers] [Merge] ~pappacena/turnip:remove-py2-env into turnip:master
Thiago F. Pappacena has proposed merging ~pappacena/turnip:remove-py2-env into
turnip:master.
Commit message:
Removing python2 env and test execution
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~pappacena/turnip/+git/turnip/+merge/401567
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~pappacena/turnip:remove-py2-env into turnip:master.
diff --git a/Makefile b/Makefile
index 26bf506..cb4204c 100644
--- a/Makefile
+++ b/Makefile
@@ -2,7 +2,6 @@
# GNU Affero General Public License version 3 (see the file LICENSE).
ENV := $(CURDIR)/env
-PY2_ENV := $(CURDIR)/py2env
PIP_CACHE = $(CURDIR)/pip-cache
PYTHON := $(ENV)/bin/python
@@ -78,20 +77,12 @@ bootstrap-test:
-sudo rabbitmqctl add_vhost turnip-test-vhost
-sudo rabbitmqctl set_permissions -p "turnip-test-vhost" "guest" ".*" ".*" ".*"
-run-test: $(ENV) bootstrap-test
+test: $(ENV) bootstrap-test
$(PYTHON) -m unittest discover $(ARGS) turnip
-test: test-python3 test-python2
-
-test-python3:
- $(MAKE) run-test VENV_ARGS="-p python3"
-
-test-python2:
- $(MAKE) run-test VENV_ARGS="-p python2" ENV="$(PY2_ENV)"
-
clean:
find turnip -name '*.py[co]' -exec rm '{}' \;
- rm -rf $(ENV) $(PY2_ENV) $(PIP_CACHE)
+ rm -rf $(ENV) $(PIP_CACHE)
rm -f turnip/version_info.py
dist:
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:oci-bug-indexes-backport into launchpad:master
The proposal to merge ~pappacena/launchpad:oci-bug-indexes-backport into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401488 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:oci-bug-indexes-backport. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:oci-bug-indexes-backport into launchpad:master
Review: Approve Self-approving already applied db patch (already available in db-devel). -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401488 Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:oci-bug-indexes-backport. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:oci-bug-indexes-backport into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:oci-bug-indexes-backport into launchpad:master. Commit message: Backporting OCI bug support indexes creation from db-devel (already applied in prod) Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401488 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:oci-bug-indexes-backport into launchpad:master. diff --git a/database/schema/patch-2210-22-1.sql b/database/schema/patch-2210-22-1.sql new file mode 100644 index 000..7f4cedd --- /dev/null +++ b/database/schema/patch-2210-22-1.sql @@ -0,0 +1,135 @@ +-- Copyright 2020 Canonical Ltd. This software is licensed under the +-- GNU Affero General Public License version 3 (see the file LICENSE). + +SET client_min_messages=ERROR; + +-- Validate check constraint. +ALTER TABLE BugTask VALIDATE CONSTRAINT bugtask_assignment_checks; + +ALTER TABLE BugSummary VALIDATE CONSTRAINT bugtask_assignment_checks; + +-- BugTask indexes. +CREATE UNIQUE INDEX bugtask__ociproject__bug__key +ON BugTask (ociproject, bug) +WHERE ociproject IS NOT NULL; +CREATE UNIQUE INDEX bugtask__ociprojectseries__bug__key +ON BugTask (ociprojectseries, bug) +WHERE ociprojectseries IS NOT NULL; + +-- BugTaskFlat indexes. +CREATE INDEX bugtaskflat__ociproject__bug__idx +ON BugTaskFlat (ociproject, bug) +WHERE ociproject IS NOT NULL; +CREATE INDEX bugtaskflat__ociproject__date_closed__bug__idx +ON BugTaskFlat (ociproject, date_closed, bug DESC) +WHERE ociproject IS NOT NULL; +CREATE INDEX bugtaskflat__ociproject__date_last_updated__idx +ON BugTaskFlat (ociproject, date_last_updated) +WHERE ociproject IS NOT NULL; +CREATE INDEX bugtaskflat__ociproject__datecreated__idx +ON BugTaskFlat (ociproject, datecreated) +WHERE ociproject IS NOT NULL; +CREATE INDEX bugtaskflat__ociproject__heat__bug__idx +ON BugTaskFlat (ociproject, heat, bug DESC) +WHERE ociproject IS NOT NULL; +CREATE INDEX bugtaskflat__ociproject__importance__bug__idx +ON BugTaskFlat (ociproject, importance, bug DESC) +WHERE ociproject IS NOT NULL; +CREATE INDEX bugtaskflat__ociproject__latest_patch_uploaded__bug__idx +ON BugTaskFlat (ociproject, latest_patch_uploaded, bug DESC) +WHERE ociproject IS NOT NULL; +CREATE INDEX bugtaskflat__ociproject__status__bug__idx +ON BugTaskFlat (ociproject, status, bug DESC) +WHERE ociproject IS NOT NULL; + +CREATE INDEX bugtaskflat__ociprojectseries__bug__idx +ON BugTaskFlat (ociprojectseries, bug) +WHERE ociprojectseries IS NOT NULL; +CREATE INDEX bugtaskflat__ociprojectseries__date_closed__bug__idx +ON BugTaskFlat (ociprojectseries, date_closed, bug DESC) +WHERE ociprojectseries IS NOT NULL; +CREATE INDEX bugtaskflat__ociprojectseries__date_last_updated__idx +ON BugTaskFlat (ociprojectseries, date_last_updated) +WHERE ociprojectseries IS NOT NULL; +CREATE INDEX bugtaskflat__ociprojectseries__datecreated__idx +ON BugTaskFlat (ociprojectseries, datecreated) +WHERE ociprojectseries IS NOT NULL; +CREATE INDEX bugtaskflat__ociprojectseries__heat__bug__idx +ON BugTaskFlat (ociprojectseries, heat, bug DESC) +WHERE ociprojectseries IS NOT NULL; +CREATE INDEX bugtaskflat__ociprojectseries__importance__bug__idx +ON BugTaskFlat (ociprojectseries, importance, bug DESC) +WHERE ociprojectseries IS NOT NULL; +CREATE INDEX bugtaskflat__ociprojectseries__latest_patch_uploaded__bug__idx +ON BugTaskFlat (ociprojectseries, latest_patch_uploaded, bug DESC) +WHERE ociprojectseries IS NOT NULL; +CREATE INDEX bugtaskflat__ociprojectseries__status__bug__idx +ON BugTaskFlat (ociprojectseries, status, bug DESC) +WHERE ociprojectseries IS NOT NULL; + + +-- BugSummary indexes. +CREATE INDEX bugsummary__ociproject__idx +ON BugSummary (ociproject) +WHERE ociproject IS NOT NULL; +CREATE INDEX bugsummary__ociprojectseries__idx +ON BugSummary (ociprojectseries) +WHERE ociprojectseries IS NOT NULL; + + +-- Replacing previously renamed indexes. +CREATE UNIQUE INDEX bugtask_distinct_sourcepackage_assignment +ON BugTask ( +bug, +COALESCE(sourcepackagename, -1), +COALESCE(distroseries, -1), +COALESCE(distribution, -1) +) +WHERE +product IS NULL +AND productseries IS NULL +AND ociproject IS NULL +AND ociprojectseries IS NULL; +DROP INDEX old__bugtask_distinct_sourcepackage_assignment; + + +CREATE UNIQUE INDEX bugtask__product__bug__key +ON BugTask (product, bug) +WHERE +product IS NOT NULL +AND ociproject IS NULL +AND ociprojectseries IS NULL; +DROP INDEX old__bugtask__product__bug__key; + + +CREATE UNIQUE INDEX bugsummary__unique +ON BugSummary ( +COALESCE(product, -1), +COALESCE(productseries, -1), +COALESCE
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:testfix-ignore-unset-error-on-mp-conversation into launchpad:master
The proposal to merge ~pappacena/launchpad:testfix-ignore-unset-error-on-mp-conversation into launchpad:master has been updated. Status: Work in progress => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401485 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:testfix-ignore-unset-error-on-mp-conversation. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:gitref-and-mp-pages-without-oops-on-failure into launchpad:master
The proposal to merge ~pappacena/launchpad:gitref-and-mp-pages-without-oops-on-failure into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401248 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:gitref-and-mp-pages-without-oops-on-failure. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:gitref-and-mp-pages-without-oops-on-failure into launchpad:master
Pushed the requested changes. Merging this.
Diff comments:
> diff --git a/lib/lp/code/browser/branchmergeproposal.py
> b/lib/lp/code/browser/branchmergeproposal.py
> index 1662539..659a5c9 100644
> --- a/lib/lp/code/browser/branchmergeproposal.py
> +++ b/lib/lp/code/browser/branchmergeproposal.py
> @@ -133,6 +134,12 @@ from lp.services.webapp.interfaces import ILaunchBag
> from lp.services.webapp.menu import NavigationMenu
>
>
> +GIT_HOSTING_ERROR_MSG = (
> +"There was an error fetching revisions from git servers. "
> +"Please, try again in some minutes. If the problem persists, "
Ok!
> +"contact Launchpad support.")
Ok!
> +
> +
> def latest_proposals_for_each_branch(proposals):
> """Returns the most recent merge proposals for any particular branch.
>
> @@ -374,6 +385,16 @@ class UnmergedRevisionsMixin:
> self.context.merge_source.identity,
> self.context.merge_target.identity))
> return []
> +except GitRepositoryScanFault as e:
> +log.exception("Error scanning git repository: %s" % e)
> +self._unlanded_revisions_message = GIT_HOSTING_ERROR_MSG
> +return []
> +
> +@property
> +def commit_infos_message(self):
> +if self._unlanded_revisions_message is None:
> +self.unlanded_revisions
Ok!
> +return self._unlanded_revisions_message
>
> @property
> def pending_updates(self):
> @@ -710,6 +736,12 @@ class BranchMergeProposalView(LaunchpadFormView,
> UnmergedRevisionsMixin,
> self._populate_previewdiffs(comments)
> return CodeReviewConversation(comments)
>
> +@property
> +def conversation_message(self):
> +if self._conversation_message is None:
> +self.conversation
Ok!
> +return self._conversation_message
> +
> def _populate_previewdiffs(self, comments):
> """Lookup and populate caches for 'previewdiff_id'.
>
> diff --git a/lib/lp/code/browser/gitref.py b/lib/lp/code/browser/gitref.py
> index ff01a72..89577c9 100644
> --- a/lib/lp/code/browser/gitref.py
> +++ b/lib/lp/code/browser/gitref.py
> @@ -219,9 +226,27 @@ class GitRefView(LaunchpadView, HasSnapsViewMixin):
>
> @cachedproperty
> def commit_infos(self):
> -return self.context.getLatestCommits(
> -extended_details=True, user=self.user, handle_timeout=True,
> -logger=log)
> +try:
> +self._commit_info_message = ''
> +return self.context.getLatestCommits(
> +extended_details=True, user=self.user, handle_timeout=True,
> +logger=log)
> +except GitRepositoryScanFault as e:
> +log.error("There was an error fetching git commit info: %s" % e)
> +self._commit_info_message = (
> +"There was an error while fetching commit information from "
> +"code hosting service. Please, try again in some minutes. "
> +"If the problem persists, contact Launchpad support.")
> +return []
> +except Exception as e:
> +log.error("There was an error scanning %s: (%s) %s" %
> + (self.context, e.__class__, e))
> +raise
> +
> +def commit_infos_message(self):
> +if self._commit_info_message is None:
> +self.commit_infos
Ok!
> +return self._commit_info_message
>
> @property
> def recipes_link(self):
> diff --git a/lib/lp/code/templates/branchmergeproposal-index.pt
> b/lib/lp/code/templates/branchmergeproposal-index.pt
> index d8eb68f..551ab1c 100644
> --- a/lib/lp/code/templates/branchmergeproposal-index.pt
> +++ b/lib/lp/code/templates/branchmergeproposal-index.pt
> @@ -132,6 +132,9 @@
>
> tal:content="structure view/conversation/@@+render"/>
> +
Ok!
> +
> +
>
>
>
--
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401248
Your team Launchpad code reviewers is subscribed to branch
~pappacena/launchpad:gitref-and-mp-pages-without-oops-on-failure.
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:delete-oci-project into launchpad:master
Thiago F. Pappacena has proposed merging
~pappacena/launchpad:delete-oci-project into launchpad:master.
Commit message:
Adding API to delete OCI projects
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1925079 in Launchpad itself: "Add possibility to delete a OCI project"
https://bugs.launchpad.net/launchpad/+bug/1925079
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401424
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~pappacena/launchpad:delete-oci-project into launchpad:master.
diff --git a/lib/lp/registry/interfaces/ociproject.py b/lib/lp/registry/interfaces/ociproject.py
index bb0a785..b1ea1a2 100644
--- a/lib/lp/registry/interfaces/ociproject.py
+++ b/lib/lp/registry/interfaces/ociproject.py
@@ -7,6 +7,7 @@ from __future__ import absolute_import, print_function, unicode_literals
__metaclass__ = type
__all__ = [
+'CannotDeleteOCIProject',
'IOCIProject',
'IOCIProjectSet',
'OCI_PROJECT_ALLOW_CREATE',
@@ -16,6 +17,7 @@ __all__ = [
from lazr.restful.declarations import (
call_with,
error_status,
+export_destructor_operation,
export_factory_operation,
exported,
exported_as_webservice_entry,
@@ -60,6 +62,11 @@ from lp.services.fields import (
OCI_PROJECT_ALLOW_CREATE = 'oci.project.create.enabled'
+@error_status(http_client.BAD_REQUEST)
+class CannotDeleteOCIProject(Exception):
+"""The OCIProject cannnot be deleted."""
+
+
class IOCIProjectView(IHasGitRepositories, Interface):
"""IOCIProject attributes that require launchpad.View permission."""
@@ -160,6 +167,16 @@ class IOCIProjectEdit(Interface):
def setOfficialRecipeStatus(recipe, status):
"""Change whether an OCI Recipe is official or not for this project."""
+@export_destructor_operation()
+@operation_for_version('devel')
+def destroySelf():
+"""Delete this OCI project.
+
+Any OCI recipe and git repository related to this OCI project should
+be deleted beforehand. OCIProjectSeries objects are automatically
+deleted.
+"""
+
class IOCIProjectLegitimate(Interface):
"""IOCIProject methods that require launchpad.AnyLegitimatePerson
diff --git a/lib/lp/registry/interfaces/ociprojectseries.py b/lib/lp/registry/interfaces/ociprojectseries.py
index d51eebd..ee20e4b 100644
--- a/lib/lp/registry/interfaces/ociprojectseries.py
+++ b/lib/lp/registry/interfaces/ociprojectseries.py
@@ -1,4 +1,4 @@
-# Copyright 2019-2020 Canonical Ltd. This software is licensed under the
+# Copyright 2019-2021 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Interfaces to allow bug filing on multiple versions of an OCI Project."""
@@ -77,6 +77,9 @@ class IOCIProjectSeriesEditableAttributes(Interface):
class IOCIProjectSeriesEdit(Interface):
"""IOCIProjectSeries attributes that require launchpad.Edit permission."""
+def destroySelf():
+"""Delete this OCI project series."""
+
@exported_as_webservice_entry(
publish_web_link=True, as_of="devel", singular_name="oci_project_series")
diff --git a/lib/lp/registry/model/ociproject.py b/lib/lp/registry/model/ociproject.py
index e98b3ea..da0bb98 100644
--- a/lib/lp/registry/model/ociproject.py
+++ b/lib/lp/registry/model/ociproject.py
@@ -46,6 +46,7 @@ from lp.code.model.branchnamespace import (
from lp.oci.interfaces.ocirecipe import IOCIRecipeSet
from lp.registry.interfaces.distribution import IDistribution
from lp.registry.interfaces.ociproject import (
+CannotDeleteOCIProject,
IOCIProject,
IOCIProjectSet,
)
@@ -297,6 +298,28 @@ class OCIProject(BugTargetBase, StormBase):
namespace = getUtility(IGitNamespaceSet).get(person, oci_project=self)
return namespace.name
+def destroySelf(self):
+"""See `IOCIProject`."""
+from lp.oci.model.ocirecipe import OCIRecipe
+from lp.code.model.gitrepository import GitRepository
+
+exists_recipes = not IStore(OCIRecipe).find(
+OCIRecipe,
+OCIRecipe.oci_project == self).is_empty()
+if exists_recipes:
+raise CannotDeleteOCIProject("This OCI recipe contains recipes.")
+
+git_repos = IStore(GitRepository).find(
+GitRepository, GitRepository.oci_project == self)
+if not git_repos.is_empty():
+repos = ", ".join(repo.display_name for repo in git_repos)
+raise CannotDeleteOCIProject(
+"There are git repositories associated with this OCI project:
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:gitref-and-mp-pages-without-oops-on-failure into launchpad:master
Thiago F. Pappacena has proposed merging
~pappacena/launchpad:gitref-and-mp-pages-without-oops-on-failure into
launchpad:master.
Commit message:
Avoiding crashes on MP and gitrefs pages when something goes wrong on git code
hosting
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1923895 in Launchpad itself: "OOPS on MP pages when Turnip is not fully
available"
https://bugs.launchpad.net/launchpad/+bug/1923895
For more details, see:
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/401248
--
Your team Launchpad code reviewers is requested to review the proposed merge of
~pappacena/launchpad:gitref-and-mp-pages-without-oops-on-failure into
launchpad:master.
diff --git a/lib/lp/code/browser/branchmergeproposal.py b/lib/lp/code/browser/branchmergeproposal.py
index 1662539..659a5c9 100644
--- a/lib/lp/code/browser/branchmergeproposal.py
+++ b/lib/lp/code/browser/branchmergeproposal.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2019 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Views, navigation and actions for BranchMergeProposals."""
@@ -88,6 +88,7 @@ from lp.code.errors import (
BranchMergeProposalExists,
ClaimReviewFailed,
DiffNotFound,
+GitRepositoryScanFault,
InvalidBranchMergeProposal,
WrongBranchMergeProposal,
)
@@ -133,6 +134,12 @@ from lp.services.webapp.interfaces import ILaunchBag
from lp.services.webapp.menu import NavigationMenu
+GIT_HOSTING_ERROR_MSG = (
+"There was an error fetching revisions from git servers. "
+"Please, try again in some minutes. If the problem persists, "
+"contact Launchpad support.")
+
+
def latest_proposals_for_each_branch(proposals):
"""Returns the most recent merge proposals for any particular branch.
@@ -359,10 +366,14 @@ class BranchMergeProposalActionNavigationMenu(NavigationMenu,
class UnmergedRevisionsMixin:
"""Provides the methods needed to show unmerged revisions."""
+# This is set at self.unlanded_revisions, and should be used at view level
+# as self.commit_infos_message (see git-macros.pt).
+_unlanded_revisions_message = None
@cachedproperty
def unlanded_revisions(self):
"""Return the unlanded revisions from the source branch."""
+self._unlanded_revisions_message = ''
with reduced_timeout(1.0, webapp_max=5.0):
try:
return self.context.getUnlandedSourceBranchRevisions()
@@ -374,6 +385,16 @@ class UnmergedRevisionsMixin:
self.context.merge_source.identity,
self.context.merge_target.identity))
return []
+except GitRepositoryScanFault as e:
+log.exception("Error scanning git repository: %s" % e)
+self._unlanded_revisions_message = GIT_HOSTING_ERROR_MSG
+return []
+
+@property
+def commit_infos_message(self):
+if self._unlanded_revisions_message is None:
+self.unlanded_revisions
+return self._unlanded_revisions_message
@property
def pending_updates(self):
@@ -664,6 +685,7 @@ class BranchMergeProposalView(LaunchpadFormView, UnmergedRevisionsMixin,
merge_proposal = self.context
with reduced_timeout(1.0, webapp_max=5.0):
try:
+self._conversation_message = ''
groups = list(merge_proposal.getRevisionsSinceReviewStart())
except TimeoutError:
log.exception(
@@ -673,6 +695,10 @@ class BranchMergeProposalView(LaunchpadFormView, UnmergedRevisionsMixin,
merge_proposal.merge_source.identity,
merge_proposal.merge_target.identity))
groups = []
+except GitRepositoryScanFault as e:
+log.exception("Error scanning git repository: %s" % e)
+self._conversation_message = GIT_HOSTING_ERROR_MSG
+groups = []
source = merge_proposal.merge_source
if IBranch.providedBy(source):
source = DecoratedBranch(source)
@@ -710,6 +736,12 @@ class BranchMergeProposalView(LaunchpadFormView, UnmergedRevisionsMixin,
self._populate_previewdiffs(comments)
return CodeReviewConversation(comments)
+@property
+def conversation_message(self):
+if self._conversation_message is None:
+self.conversation
+return self._conversation_message
+
def _populate_previewdiffs(self, comments):
"""Lookup and populate caches for 'previewdiff_id'.
diff --git a/lib/lp/code/browser/gitref.py b/lib/
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:oci-bug-add-oci-columns into launchpad:db-devel
The proposal to merge ~pappacena/launchpad:oci-bug-add-oci-columns into launchpad:db-devel has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/393460 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:oci-bug-add-oci-columns. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:bug-oci-project-navigation into launchpad:master
As discussed elsewhere, I agree on keeping the UI as it is today, and using the distribution/package field to select OCI project for OCI-related distributions. OCI projects based on projects should be a future work, if needed. My initial idea was changing BugTaskTargetWidget's vocabulary itself, but the code to do that felt a bit hacky, patching internal things on the widget and Choice fields. It didn't look good, so I reverted that, and added DistributionPackageVocabulary to be a dynamic vocabulary wrapper. This new vocabulary selects either OCI projects or binary/source packages from distributions, depending on the selected distribution's default_traversal_policy. It required a minor refactoring on LaunchpadTargetWidget, and some OCI-specific implementation on BugTaskTargetWidget. I've added some TestBugTaskEditView.test_retarget_* tests to cover edge cases on migrating bugtasks between normal distributions and OCI distributions, which turned out the biggest source of problems when doing manual tests. I think this is good for another round of review now. -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/394181 Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:bugtask-oci-project. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:oci-bug-indexes into launchpad:db-devel
Diff comments: > diff --git a/database/schema/patch-2210-22-1.sql > b/database/schema/patch-2210-22-1.sql > new file mode 100644 > index 000..4a00207 > --- /dev/null > +++ b/database/schema/patch-2210-22-1.sql > @@ -0,0 +1,135 @@ > +-- Copyright 2020 Canonical Ltd. This software is licensed under the > +-- GNU Affero General Public License version 3 (see the file LICENSE). > + > +SET client_min_messages=ERROR; > + > +-- Validate check constraint. > +ALTER TABLE BugTask VALIDATE CONSTRAINT bugtask_assignment_checks; > + > +ALTER TABLE BugSummary VALIDATE CONSTRAINT bugtask_assignment_checks; > + > +-- BugTask indexes. > +CREATE UNIQUE INDEX bugtask__ociproject__bug__idx > +ON BugTask (ociproject, bug) > +WHERE ociproject IS NOT NULL; > +CREATE UNIQUE INDEX bugtask__ociprojectseries__bug__idx > +ON BugTask (ociprojectseries, bug) > +WHERE ociprojectseries IS NOT NULL; Fixing it. > + > +-- BugTaskFlat indexes. > +CREATE INDEX bugtaskflat__ociproject__bug__idx > +ON BugTaskFlat (ociproject, bug) > +WHERE ociproject IS NOT NULL; > +CREATE INDEX bugtaskflat__ociproject__date_closed__bug__idx > +ON BugTaskFlat (ociproject, date_closed, bug DESC) > +WHERE ociproject IS NOT NULL; > +CREATE INDEX bugtaskflat__ociproject__date_last_updated__idx > +ON BugTaskFlat (ociproject, date_last_updated) > +WHERE ociproject IS NOT NULL; > +CREATE INDEX bugtaskflat__ociproject__datecreated__idx > +ON BugTaskFlat (ociproject, datecreated) > +WHERE ociproject IS NOT NULL; > +CREATE INDEX bugtaskflat__ociproject__heat__bug__idx > +ON BugTaskFlat (ociproject, heat, bug DESC) > +WHERE ociproject IS NOT NULL; > +CREATE INDEX bugtaskflat__ociproject__importance__bug__idx > +ON BugTaskFlat (ociproject, importance, bug DESC) > +WHERE ociproject IS NOT NULL; > +CREATE INDEX bugtaskflat__ociproject__latest_patch_uploaded__bug__idx > +ON BugTaskFlat (ociproject, latest_patch_uploaded, bug DESC) > +WHERE ociproject IS NOT NULL; > +CREATE INDEX bugtaskflat__ociproject__status__bug__idx > +ON BugTaskFlat (ociproject, status, bug DESC) > +WHERE ociproject IS NOT NULL; > + > +CREATE INDEX bugtaskflat__ociprojectseries__bug__idx > +ON BugTaskFlat (ociprojectseries, bug) > +WHERE ociprojectseries IS NOT NULL; > +CREATE INDEX bugtaskflat__ociprojectseries__date_closed__bug__idx > +ON BugTaskFlat (ociprojectseries, date_closed, bug DESC) > +WHERE ociprojectseries IS NOT NULL; > +CREATE INDEX bugtaskflat__ociprojectseries__date_last_updated__idx > +ON BugTaskFlat (ociprojectseries, date_last_updated) > +WHERE ociprojectseries IS NOT NULL; > +CREATE INDEX bugtaskflat__ociprojectseries__datecreated__idx > +ON BugTaskFlat (ociprojectseries, datecreated) > +WHERE ociprojectseries IS NOT NULL; > +CREATE INDEX bugtaskflat__ociprojectseries__heat__bug__idx > +ON BugTaskFlat (ociprojectseries, heat, bug DESC) > +WHERE ociprojectseries IS NOT NULL; > +CREATE INDEX bugtaskflat__ociprojectseries__importance__bug__idx > +ON BugTaskFlat (ociprojectseries, importance, bug DESC) > +WHERE ociprojectseries IS NOT NULL; > +CREATE INDEX bugtaskflat__ociprojectseries__latest_patch_uploaded__bug__idx > +ON BugTaskFlat (ociprojectseries, latest_patch_uploaded, bug DESC) > +WHERE ociprojectseries IS NOT NULL; > +CREATE INDEX bugtaskflat__ociprojectseries__status__bug__idx > +ON BugTaskFlat (ociprojectseries, status, bug DESC) > +WHERE ociprojectseries IS NOT NULL; > + > + > +-- BugSummary indexes. > +CREATE INDEX bugsummary__ociproject__idx > +ON BugSummary (ociproject) > +WHERE ociproject IS NOT NULL; > +CREATE INDEX bugsummary__ociprojectseries__idx > +ON BugSummary (ociprojectseries) > +WHERE ociprojectseries IS NOT NULL; > + > + > +-- Replacing previously renamed indexes. > +CREATE UNIQUE INDEX bugtask_distinct_sourcepackage_assignment > +ON BugTask ( > +bug, > +COALESCE(sourcepackagename, -1), > +COALESCE(distroseries, -1), > +COALESCE(distribution, -1) > +) > +WHERE > +product IS NULL > +AND productseries IS NULL > +AND ociproject IS NULL > +AND ociprojectseries IS NULL; > +DROP INDEX old__bugtask_distinct_sourcepackage_assignment; > + > + > +CREATE UNIQUE INDEX bugtask__product__bug__key > +ON BugTask (product, bug) > +WHERE > +product IS NOT NULL > +AND ociproject IS NULL > +AND ociprojectseries IS NULL; > +DROP INDEX old__bugtask__product__bug__key; > + > + > +CREATE UNIQUE INDEX bugsummary__unique > +ON BugSummary ( > +COALESCE(product, -1), > +COALESCE(productseries, -1), > +COALESCE(distribution, -1), > +COALESCE(distroseries, -1), > +COALESCE(sourcepackagename, -1), > +COALESCE(ociproject, -1), > +COALESCE(ociprojectseries, -1), > +status, > +importance, > +has_patch, > +COALESCE(tag,
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:ocirecipe-sharing-lists into launchpad:master
The proposal to merge ~pappacena/launchpad:ocirecipe-sharing-lists into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400059 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:ocirecipe-edit-info-type-ui. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:ocirecipe-sharing-lists into launchpad:master
Pushed text and style fixes, and a pre-existing bug on team members count.
Diff comments:
> diff --git a/lib/lp/registry/javascript/sharing/sharingdetails.js
> b/lib/lp/registry/javascript/sharing/sharingdetails.js
> index 0ac4f32..22db012 100644
> --- a/lib/lp/registry/javascript/sharing/sharingdetails.js
> +++ b/lib/lp/registry/javascript/sharing/sharingdetails.js
> @@ -232,7 +304,7 @@ ns.SharingDetailsTable =
> Y.Base.create('sharingDetailsTable', Y.Widget, [], {
> .appendChild('')
> .setContent(
> "There are no shared bugs, Bazaar branches, " +
> -"Git repositories, or blueprints.");
> +"Git repositories, Snaps, OCI recipes or
> blueprints.");
Done.
> }
> };
> var anim_duration = this.get('anim_duration');
> diff --git a/lib/lp/registry/templates/pillar-sharing-details.pt
> b/lib/lp/registry/templates/pillar-sharing-details.pt
> index dc907dd..8a9a2fe 100644
> --- a/lib/lp/registry/templates/pillar-sharing-details.pt
> +++ b/lib/lp/registry/templates/pillar-sharing-details.pt
> @@ -26,21 +26,32 @@
>
>
>
> -
> - 0 bugs,
> - 0
> Bazaar branches,
> - replace="view/shared_gitrepositories_count">0 Git
> repositories,
> - 0 snaps,
> - and - replace="view/shared_specifications_count">0
> - blueprints shared with
> - grantee.
> -
>
> 3 team members can view these bugs,
> -Bazaar branches, Git repositories, and blueprints.
> +Bazaar branches, Git repositories, snaps recipes, OCI recipes and
Even looking at the code, I missed that hardcoded number.
Replaced the text and added a fix for the count, using
`team.active_member_count`.
> +blueprints.
>
> -
> + Shared with replace="view/person/displayname">grantee:
> +
There is another CSS class that better suites this. After using that one, it
looks like this:
https://private-fileshare.canonical.com/~pappacena/screenshots/private-oci-recipe/bullet-sharing.png
> +
> + bugs
> +
> +
> + Bazaar branches
> +
> +
> + Git
> repositories
> +
> +
> + OCI recipes
> +
> +
> + snap recipes
> +
> +
> + blueprints
> +
> +
>
>
>
--
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400059
Your team Launchpad code reviewers is subscribed to branch
~pappacena/launchpad:ocirecipe-edit-info-type-ui.
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:oci-recipes-link-for-teams into launchpad:master
The proposal to merge ~pappacena/launchpad:oci-recipes-link-for-teams into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400897 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:oci-recipes-link-for-teams. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:oci-recipes-link-for-teams into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:oci-recipes-link-for-teams into launchpad:master. Commit message: Moving "View OCI recipe" link to correct place and adding it to team page Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400897 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:oci-recipes-link-for-teams into launchpad:master. diff --git a/lib/lp/oci/browser/hasocirecipes.py b/lib/lp/oci/browser/hasocirecipes.py new file mode 100644 index 000..2df4148 --- /dev/null +++ b/lib/lp/oci/browser/hasocirecipes.py @@ -0,0 +1,27 @@ +# Copyright 2021 Canonical Ltd. This software is licensed under the +# GNU Affero General Public License version 3 (see the file LICENSE). + +"""Mixins for browser classes for objects related to OCI recipe.""" + +from __future__ import absolute_import, print_function, unicode_literals + +__metaclass__ = type +__all__ = [ +'HasOCIRecipesMenuMixin', +] + +from lp.oci.interfaces.ocirecipe import IOCIRecipeSet +from zope.component import getUtility + +from lp.services.webapp import Link + + +class HasOCIRecipesMenuMixin: +"""A mixin for context menus for objects that has OCI recipes.""" + +def view_oci_recipes(self): +target = '+oci-recipes' +text = 'View OCI recipes' +enabled = not getUtility(IOCIRecipeSet).findByContext( +self.context, visible_by_user=self.user).is_empty() +return Link(target, text, enabled=enabled, icon='info') diff --git a/lib/lp/registry/browser/person.py b/lib/lp/registry/browser/person.py index 6d5d8da..2b71694 100644 --- a/lib/lp/registry/browser/person.py +++ b/lib/lp/registry/browser/person.py @@ -138,11 +138,9 @@ from lp.code.browser.sourcepackagerecipelisting import HasRecipesMenuMixin from lp.code.errors import InvalidNamespace from lp.code.interfaces.branchnamespace import IBranchNamespaceSet from lp.code.interfaces.gitlookup import IGitTraverser +from lp.oci.browser.hasocirecipes import HasOCIRecipesMenuMixin from lp.oci.interfaces.ocipushrule import IOCIPushRuleSet -from lp.oci.interfaces.ocirecipe import ( -IOCIRecipe, -IOCIRecipeSet, -) +from lp.oci.interfaces.ocirecipe import IOCIRecipe from lp.oci.interfaces.ociregistrycredentials import ( IOCIRegistryCredentialsSet, OCIRegistryCredentialsAlreadyExist, @@ -777,13 +775,6 @@ class CommonMenuLinks: enabled = user_can_edit_credentials_for_owner(self.context, self.user) return Link(target, text, enabled=enabled, icon='info') -def oci_recipes(self): -target = '+oci-recipes' -text = 'OCI recipes' -enabled = not getUtility(IOCIRecipeSet).findByContext( -self.context, visible_by_user=self.user).is_empty() -return Link(target, text, enabled=enabled, icon='info') - class PersonMenuMixin(CommonMenuLinks): @@ -818,8 +809,8 @@ class PersonMenuMixin(CommonMenuLinks): return Link(target, text, icon='edit') -class PersonOverviewMenu(ApplicationMenu, PersonMenuMixin, - HasRecipesMenuMixin, HasSnapsMenuMixin): +class PersonOverviewMenu(ApplicationMenu, PersonMenuMixin, HasRecipesMenuMixin, + HasSnapsMenuMixin, HasOCIRecipesMenuMixin): usedfor = IPerson facet = 'overview' @@ -848,10 +839,10 @@ class PersonOverviewMenu(ApplicationMenu, PersonMenuMixin, 'ppa', 'oauth_tokens', 'oci_registry_credentials', -'oci_recipes', 'related_software_summary', 'view_recipes', 'view_snaps', +'view_oci_recipes', 'subscriptions', 'structural_subscriptions', ] diff --git a/lib/lp/registry/browser/team.py b/lib/lp/registry/browser/team.py index 93f121c..d419603 100644 --- a/lib/lp/registry/browser/team.py +++ b/lib/lp/registry/browser/team.py @@ -1,4 +1,4 @@ -# Copyright 2009-2020 Canonical Ltd. This software is licensed under the +# Copyright 2009-2021 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). __metaclass__ = type @@ -94,6 +94,7 @@ from lp.app.widgets.itemswidgets import ( from lp.app.widgets.owner import HiddenUserWidget from lp.app.widgets.popup import PersonPickerWidget from lp.code.browser.sourcepackagerecipelisting import HasRecipesMenuMixin +from lp.oci.browser.hasocirecipes import HasOCIRecipesMenuMixin from lp.registry.browser.branding import BrandingChangeView from lp.registry.browser.mailinglists import enabled_with_active_mailing_list from lp.registry.browser.objectreassignment import ObjectReassignmentView @@ -1623,7 +1624,7 @@ class TeamMenuMixin(PPANavigationMenuMixIn, CommonMenuLinks): class TeamOverviewMenu(ApplicationMenu, TeamMen
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:ocirecipe-list-on-person into launchpad:master
The proposal to merge ~pappacena/launchpad:ocirecipe-list-on-person into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400214 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:ocirecipe-filter-private. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:ocirecipe-list-on-person into launchpad:master
Pushed requested changes.
Diff comments:
> diff --git a/lib/lp/oci/browser/ocirecipe.py b/lib/lp/oci/browser/ocirecipe.py
> index 5eee7e3..4d526ff 100644
> --- a/lib/lp/oci/browser/ocirecipe.py
> +++ b/lib/lp/oci/browser/ocirecipe.py
> @@ -204,7 +204,9 @@ class OCIRecipeContextMenu(ContextMenu):
>
>
> class OCIProjectRecipesView(LaunchpadView):
> -"""Default view for the list of OCI recipes of an OCI project."""
> +"""Default view for the list of OCI recipes of a context (OCI project
> +or Person).
> +"""
Ok! Renaming it!
> page_title = 'Recipes'
>
> @property
> diff --git a/lib/lp/oci/model/ocirecipe.py b/lib/lp/oci/model/ocirecipe.py
> index 2a5598a..1397837 100644
> --- a/lib/lp/oci/model/ocirecipe.py
> +++ b/lib/lp/oci/model/ocirecipe.py
> @@ -875,6 +877,14 @@ class OCIRecipeSet:
> OCIRecipe.oci_project == oci_project,
> get_ocirecipe_privacy_filter(visible_by_user))
>
> +def findByContext(self, context, visible_by_user):
> +if IPerson.providedBy(context):
> +return self.findByOwner(context).find(
> +get_ocirecipe_privacy_filter(visible_by_user))
> +if IOCIProject.providedBy(context):
Ok!
> +return self.findByOCIProject(context, visible_by_user)
> +raise NotImplementedError("Unknown OCI recipe context: %s" % context)
> +
> def findByGitRepository(self, repository, paths=None):
> """See `IOCIRecipeSet`."""
> clauses = [OCIRecipe.git_repository == repository]
--
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400214
Your team Launchpad code reviewers is subscribed to branch
~pappacena/launchpad:ocirecipe-filter-private.
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:testfix-oci-branch-format into launchpad:master
Review: Approve Self-approving trivial test fix -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400728 Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:testfix-oci-branch-format. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:testfix-oci-branch-format into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:testfix-oci-branch-format into launchpad:master. Commit message: Fixing test sample OCI recipe branch Requested reviews: Launchpad code reviewers (launchpad-reviewers) For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400728 -- Your team Launchpad code reviewers is requested to review the proposed merge of ~pappacena/launchpad:testfix-oci-branch-format into launchpad:master. diff --git a/lib/lp/oci/browser/tests/test_ocirecipe.py b/lib/lp/oci/browser/tests/test_ocirecipe.py index 7547eaf..5ecc4c8 100644 --- a/lib/lp/oci/browser/tests/test_ocirecipe.py +++ b/lib/lp/oci/browser/tests/test_ocirecipe.py @@ -715,7 +715,7 @@ class TestOCIRecipeEditView(OCIConfigHelperMixin, BaseTestOCIRecipeView): registrant=self.person, pillar=pillar) [git_ref] = self.factory.makeGitRefs( owner=self.person, -paths=['/refs/heads/v2.0-20.04']) +paths=['refs/heads/v2.0-20.04']) recipe = self.factory.makeOCIRecipe( registrant=self.person, owner=self.person, oci_project=oci_project, git_ref=git_ref, ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:testfix-oci-branch-format into launchpad:master
The proposal to merge ~pappacena/launchpad:testfix-oci-branch-format into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400728 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:testfix-oci-branch-format. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:ocirecipe-sharing-lists into launchpad:master
Replied all comments. I might worth another quick review round, or at least a
validation of the screenshots.
Diff comments:
> diff --git a/lib/lp/registry/browser/tests/test_pillar_sharing.py
> b/lib/lp/registry/browser/tests/test_pillar_sharing.py
> index 493b42b..215f946 100644
> --- a/lib/lp/registry/browser/tests/test_pillar_sharing.py
> +++ b/lib/lp/registry/browser/tests/test_pillar_sharing.py
> @@ -172,8 +172,9 @@ class PillarSharingDetailsMixin:
> pillarperson.pillar.name, pillarperson.person.name)
> browser = self.getUserBrowser(user=self.owner, url=url)
> self.assertIn(
> -'There are no shared bugs, Bazaar branches, Git repositories, or
> '
> -'blueprints.', normalize_whitespace(browser.contents))
> +'There are no shared bugs, Bazaar branches, Git repositories, '
> +'Snaps, OCI recipes or blueprints.',
Ok!
> +normalize_whitespace(browser.contents))
>
> def test_init_works(self):
> # The view works with a feature flag.
> diff --git a/lib/lp/registry/javascript/sharing/sharingdetails.js
> b/lib/lp/registry/javascript/sharing/sharingdetails.js
> index 0ac4f32..60b56d5 100644
> --- a/lib/lp/registry/javascript/sharing/sharingdetails.js
> +++ b/lib/lp/registry/javascript/sharing/sharingdetails.js
> @@ -148,6 +148,58 @@ ns.SharingDetailsTable =
> Y.Base.create('sharingDetailsTable', Y.Widget, [], {
> ].join(' ');
> },
>
> +_snap_details_row_template: function() {
> +return [
> +'',
> +'',
> +'{{id}}',
> +' href="{{web_link}}">',
I couldn't find any sprite to represent snaps today, but it felt a bit strange
to not have anything there.
Anyway, I'll remove to avoid visual confusion.
> +'{{name}}',
> +'',
> +'',
> +'',
> +'',
> +' +'title="Unshare Snap {{name}} with {{displayname}}"',
> +'data-self_link="{{self_link}}" data-name="{{name}}"',
> +'data-type="snap">Remove',
> +'',
> +'',
> +' ',
> +' ',
> +' {{information_type}}',
> +' ',
> +' ',
> +''
> +].join(' ');
> +},
> +
> +_ocirecipe_details_row_template: function() {
> +return [
> +'',
> +'',
> +'{{id}}',
> +' href="{{web_link}}">',
Ok!
> +'{{name}}',
> +'',
> +'',
> +'',
> +'',
> +' +'title="Unshare OCI recipe {{name}} with {{displayname}}"',
> +'data-self_link="{{self_link}}" data-name="{{name}}"',
> +'data-type="ocirecipe">Remove',
> +'',
> +'',
> +' ',
> +' ',
> +' {{information_type}}',
> +' ',
> +' ',
> +''
> +].join(' ');
> +},
> +
> _table_body_template: function() {
> return [
> '',
> diff --git a/lib/lp/registry/templates/pillar-sharing-details.pt
> b/lib/lp/registry/templates/pillar-sharing-details.pt
> index a6f2710..0b89496 100644
> --- a/lib/lp/registry/templates/pillar-sharing-details.pt
> +++ b/lib/lp/registry/templates/pillar-sharing-details.pt
> @@ -26,20 +26,31 @@
>
>
>
> -
> - 0 bugs,
> - 0
> Bazaar branches,
> - replace="view/shared_gitrepositories_count">0 Git
> repositories,
> - and - replace="view/shared_specifications_count">0
> - blueprints shared with
> - grantee.
> -
>
> 3 team members can view these bugs,
> -Bazaar branches, Git repositories, and blueprints.
> +Bazaar branches, Git repositories, Snaps, OCI recipes and blueprints.
Ok!
>
> -
> + Shared with replace="view/person/displayname">grantee
> +
> +
> + Bugs
Ok!
> +
> +
> + Bazaar branches
> +
> +
> + Git
> repositories
> +
> +
> + OCI recipes
> +
> +
> + Snap recipes
Ok!
> +
> +
> + Blueprints
Ok!
> +
> +
Screenshots:
- Current version:
https://private-fileshare.canonical.com/~pappacena/screenshots/private-oci-recipe/old-sharing-with-user.png
- New version:
https://private-fileshare.canonical.com/~pappacena/screenshots/private-oci-recipe/sharing-with-user.png
>
>
>
--
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400059
Your team Launchpad code reviewers is subscribed to branch
~pappacena/launchpad:ocirecipe-edit-info-type-ui.
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe :
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:ocirecipe-sharing-lists into launchpad:master
The proposal to merge ~pappacena/launchpad:ocirecipe-sharing-lists into launchpad:master has been updated. Description changed to: Screenshots: - Current version: https://private-fileshare.canonical.com/~pappacena/screenshots/private-oci-recipe/old-sharing-with-user.png - New version: https://private-fileshare.canonical.com/~pappacena/screenshots/private-oci-recipe/sharing-with-user.png For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400059 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:ocirecipe-edit-info-type-ui. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:allow-delete-gitrepo-while-creating into launchpad:master
The proposal to merge ~pappacena/launchpad:allow-delete-gitrepo-while-creating into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400490 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:allow-delete-gitrepo-while-creating. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:allow-delete-gitrepo-while-creating into launchpad:master
Diff comments: > diff --git a/lib/lp/code/browser/gitrepository.py > b/lib/lp/code/browser/gitrepository.py > index 76606cd..c41cafe 100644 > --- a/lib/lp/code/browser/gitrepository.py > +++ b/lib/lp/code/browser/gitrepository.py > @@ -1420,8 +1418,6 @@ class GitRepositoryDeletionView(LaunchpadFormView): > > @property > def warning_message(self): > -if self.context.status == GitRepositoryStatus.CREATING: > -return "This repository is being created and cannot be deleted." > return None Sure. Removing it. > > -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400490 Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:allow-delete-gitrepo-while-creating. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:ocirecipe-edit-info-type-ui into launchpad:master
The proposal to merge ~pappacena/launchpad:ocirecipe-edit-info-type-ui into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/33 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:ocirecipe-privacy-banners-ui. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:testfix-product-getOCIProject-permission into launchpad:master
Thiago F. Pappacena has proposed merging ~pappacena/launchpad:testfix-product-getOCIProject-permission into launchpad:master. Commit message: Moving permission to call Product.getOCIProject from View to LimitedView Requested reviews: Colin Watson (cjwatson) For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400663 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:testfix-product-getOCIProject-permission. diff --git a/lib/lp/registry/tests/test_product.py b/lib/lp/registry/tests/test_product.py index bd40296..0f04a93 100644 --- a/lib/lp/registry/tests/test_product.py +++ b/lib/lp/registry/tests/test_product.py @@ -829,7 +829,7 @@ class TestProduct(TestCaseWithFactory): 'launchpad.LimitedView': set(( 'bugtargetdisplayname', 'display_name', 'displayname', 'drivers', 'enable_bug_expiration', 'getBugTaskWeightFunction', -'getSpecification', +'getOCIProject', 'getSpecification', 'icon', 'logo', 'name', 'official_answers', 'official_anything', 'official_blueprints', 'official_codehosting', 'official_malone', 'owner', 'parent_subscription_target', 'pillar', 'projectgroup', @@ -865,7 +865,7 @@ class TestProduct(TestCaseWithFactory): 'getEffectiveTranslationPermission', 'getExternalBugTracker', 'getFAQ', 'getFirstEntryToImport', 'getLinkedBugWatches', 'getMergeProposals', 'getMilestone', 'getMilestonesAndReleases', -'getOCIProject', 'getQuestion', 'getQuestionLanguages', +'getQuestion', 'getQuestionLanguages', 'getPackage', 'getRelease', 'getSeries', 'getSubscription', 'getSubscriptions', 'getSupportedLanguages', 'getTimeline', 'getTopContributors', 'getTopContributorsGroupedByCategory', ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:testfix-product-getOCIProject-permission into launchpad:master
The proposal to merge ~pappacena/launchpad:testfix-product-getOCIProject-permission into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/400663 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:testfix-product-getOCIProject-permission. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:ocirecipe-privacy-banners-ui into launchpad:master
The proposal to merge ~pappacena/launchpad:ocirecipe-privacy-banners-ui into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399987 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:ocirecipe-subscribe-removal-job. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:ocirecipe-privacy-banners-ui into launchpad:master
Diff comments:
> diff --git a/lib/lp/oci/interfaces/ocirecipe.py
> b/lib/lp/oci/interfaces/ocirecipe.py
> index 5ed23a7..ecc90c8 100644
> --- a/lib/lp/oci/interfaces/ocirecipe.py
> +++ b/lib/lp/oci/interfaces/ocirecipe.py
> @@ -241,6 +241,11 @@ class IOCIRecipeView(Interface):
> description=_("True if this recipe is official for its OCI
> project."),
> readonly=True)
>
> +private = Bool(
> +title=_("Is this OCI recipe private?"),
> +required=False, readonly=True,
Ok!
> +description=_("True if this recipe is private. False otherwise."))
> +
> pillar = Attribute('The pillar of this OCI recipe.')
>
> @call_with(check_permissions=True, user=REQUEST_USER)
--
https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399987
Your team Launchpad code reviewers is subscribed to branch
~pappacena/launchpad:ocirecipe-subscribe-removal-job.
___
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:ocirecipe-subscribe-removal-job into launchpad:master
The proposal to merge ~pappacena/launchpad:ocirecipe-subscribe-removal-job into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399889 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:ocirecipe-filter-private. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:ocirecipe-filter-private into launchpad:master
The proposal to merge ~pappacena/launchpad:ocirecipe-filter-private into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399886 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:ocirecipe-subscription-ui. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
Re: [Launchpad-reviewers] [Merge] ~pappacena/launchpad:ocirecipe-filter-private into launchpad:master
Diff comments: > diff --git a/lib/lp/registry/browser/ociproject.py > b/lib/lp/registry/browser/ociproject.py > index 7c3f719..2f07dd6 100644 > --- a/lib/lp/registry/browser/ociproject.py > +++ b/lib/lp/registry/browser/ociproject.py > @@ -238,12 +236,18 @@ class OCIProjectIndexView(LaunchpadView): > return urlsplit(config.codehosting.git_ssh_root).hostname > > @cachedproperty > +def official_recipes(self): > +return self.context.getOfficialRecipes(visible_by_user=self.user) I guess either way should be fine: we expect this list to be short enough to not impact memory usage; and we use it only once in the template, so we don't really need to cache it. I'll define it as `@property`, just in case someone goes crazy and creates thousands of official recipes in an OCI project. > + > +@cachedproperty > def official_recipe_count(self): > -return self.context.getOfficialRecipes().count() > +return self.context.getOfficialRecipes( > +visible_by_user=self.user).count() > > @cachedproperty > def other_recipe_count(self): > -return self.context.getUnofficialRecipes().count() > +return self.context.getUnofficialRecipes( > +visible_by_user=self.user).count() > > > class OCIProjectEditView(LaunchpadEditFormView): -- https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399886 Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:ocirecipe-subscription-ui. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
[Launchpad-reviewers] [Merge] ~pappacena/launchpad:ocirecipe-subscription-ui into launchpad:master
The proposal to merge ~pappacena/launchpad:ocirecipe-subscription-ui into launchpad:master has been updated. Status: Needs review => Approved For more details, see: https://code.launchpad.net/~pappacena/launchpad/+git/launchpad/+merge/399750 -- Your team Launchpad code reviewers is subscribed to branch ~pappacena/launchpad:ocirecipe-subscription. ___ Mailing list: https://launchpad.net/~launchpad-reviewers Post to : launchpad-reviewers@lists.launchpad.net Unsubscribe : https://launchpad.net/~launchpad-reviewers More help : https://help.launchpad.net/ListHelp
