Launchpad itself

Merge lp:~jtv/launchpad/bug-662552-view-permissions into lp:launchpad

  1. bug-662552-view-permissions
  2. Merge into devel
Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: 11818
Proposed branch: lp:~jtv/launchpad/bug-662552-view-permissions
Merge into: lp:launchpad
Diff against target: 112 lines (+19/-19)
3 files modified
lib/lp/translations/browser/pofile.py (+4/-9)
lib/lp/translations/browser/tests/test_translationmessage_view.py (+1/-2)
lib/lp/translations/browser/translationmessage.py (+14/-8)
To merge this branch: bzr merge lp:~jtv/launchpad/bug-662552-view-permissions
Reviewer Review Type Date Requested Status
Tim Penhey (community) code Approve
Launchpad code reviewers code Pending
Review via email: mp+39481@code.launchpad.net

Commit message

Re-use POFile edit permission info across message sub-views.

Description of the change

= Bug 662552: View and Permissions =

I'm cutting some dead wood out of the POFile:+translate page, which has been timing out.

One thing that can get a bit costly (especially in terms of the number of database queries issued) is checking whether the user has permission to edit the translation that's being browsed. That check is done for each of the messages displayed, normally 10 but up to 50 per page.

This branch hoists the check out of the view constructors (the batched on for the POFile as well as the single-message view) and passes the result in as an argument. That way, the POFile view can do the check once and re-use the outcome for each of the message views it creates. The code for the check is quite complicated and can involve many queries, depending on the user.

While I was at it, I also passed the POFile itself as an argument. A current problem is that TranslationMessage officially doesn't have a reference to a POFile any more, but the view code does still need that reference. We work around that in a few places with a memory-only attribute called browser_pofile. By passing the POFile as an argument, I get to eliminate one use of that malodorous attribute.

There's some legacy lint, most of which I left untouched because we're cleaning it up in other branches. None that I introduced though.

Jeroen

To post a comment you must log in.
Revision history for this message
Tim Penhey (thumper) wrote :

I don't see anything wrong with this, but I'll have to take your word that it does what it says it does.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/translations/browser/pofile.py'
2--- lib/lp/translations/browser/pofile.py 2010-10-27 18:30:31 +0000
3+++ lib/lp/translations/browser/pofile.py 2010-10-28 09:58:49 +0000
4@@ -824,20 +824,15 @@
5
6 def _buildTranslationMessageViews(self, for_potmsgsets):
7 """Build translation message views for all potmsgsets given."""
8- last = None
9 for potmsgset in for_potmsgsets:
10- assert (last is None or
11- potmsgset.getSequence(
12- self.context.potemplate) >= last.getSequence(
13- self.context.potemplate)), (
14- "POTMsgSets on page not in ascending sequence order")
15- last = potmsgset
16-
17 translationmessage = (
18 potmsgset.getCurrentTranslationMessageOrDummy(self.context))
19+ error = self.errors.get(potmsgset)
20+ can_edit = self.context.canEditTranslations(self.user)
21+
22 view = self._prepareView(
23 CurrentTranslationMessageView, translationmessage,
24- self.errors.get(potmsgset))
25+ pofile=self.context, can_edit=can_edit, error=error)
26 view.zoomed_in_view = False
27 self.translationmessage_views.append(view)
28
29
30=== modified file 'lib/lp/translations/browser/tests/test_translationmessage_view.py'
31--- lib/lp/translations/browser/tests/test_translationmessage_view.py 2010-10-04 19:50:45 +0000
32+++ lib/lp/translations/browser/tests/test_translationmessage_view.py 2010-10-28 09:58:49 +0000
33@@ -44,8 +44,7 @@
34 self.view = CurrentTranslationMessageView(
35 message, LaunchpadTestRequest(),
36 {}, dict(enumerate(message.translations)),
37- False, False, None, None, True)
38- self.view.user_is_official_translator = True
39+ False, False, None, None, True, pofile=self.pofile, can_edit=True)
40 self.view.initialize()
41
42 def _makeTranslation(self, translation=None,
43
44=== modified file 'lib/lp/translations/browser/translationmessage.py'
45--- lib/lp/translations/browser/translationmessage.py 2010-10-27 18:30:31 +0000
46+++ lib/lp/translations/browser/translationmessage.py 2010-10-28 09:58:49 +0000
47@@ -453,8 +453,9 @@
48 return False
49 return True
50
51- def _prepareView(self, view_class, current_translation_message, error):
52- """Collect data and build a TranslationMessageView for display."""
53+ def _prepareView(self, view_class, current_translation_message,
54+ pofile, can_edit, error=None):
55+ """Collect data and build a `TranslationMessageView` for display."""
56 # XXX: kiko 2006-09-27:
57 # It would be nice if we could easily check if this is being
58 # called in the right order, after _storeTranslations().
59@@ -499,7 +500,7 @@
60 current_translation_message, self.request,
61 plural_indices_to_store, translations, force_suggestion,
62 force_diverge, error, self.second_lang_code,
63- self.form_is_writeable)
64+ self.form_is_writeable, pofile=pofile, can_edit=can_edit)
65
66 #
67 # Internals
68@@ -824,8 +825,11 @@
69
70 def _initializeTranslationMessageViews(self):
71 """See `BaseTranslationView._initializeTranslationMessageViews`."""
72+ pofile = self.pofile
73+ can_edit = pofile.canEditTranslations(self.user)
74 self.translationmessage_view = self._prepareView(
75- CurrentTranslationMessageZoomedView, self.context, self.error)
76+ CurrentTranslationMessageZoomedView, self.context, pofile=pofile,
77+ can_edit=can_edit, error=self.error)
78
79 def _submitTranslations(self):
80 """See `BaseTranslationView._submitTranslations`."""
81@@ -878,7 +882,8 @@
82
83 def __init__(self, current_translation_message, request,
84 plural_indices_to_store, translations, force_suggestion,
85- force_diverge, error, second_lang_code, form_is_writeable):
86+ force_diverge, error, second_lang_code, form_is_writeable,
87+ pofile, can_edit):
88 """Primes the view with information that is gathered by a parent view.
89
90 :param plural_indices_to_store: A dictionary that indicates whether
91@@ -894,17 +899,18 @@
92 field.alternative_value.
93 :param form_is_writeable: Whether the form should accept write
94 operations
95+ :param pofile: The `POFile` that's being displayed or edited.
96+ :param can_edit: Whether the user has editing privileges on `pofile`.
97 """
98 LaunchpadView.__init__(self, current_translation_message, request)
99
100- self.pofile = self.context.browser_pofile
101+ self.pofile = pofile
102 self.plural_indices_to_store = plural_indices_to_store
103 self.translations = translations
104 self.error = error
105 self.force_suggestion = force_suggestion
106 self.force_diverge = force_diverge
107- self.user_is_official_translator = (
108- self.pofile.canEditTranslations(self.user))
109+ self.user_is_official_translator = can_edit
110 self.form_is_writeable = form_is_writeable
111 if self.context.is_imported:
112 # The imported translation matches the current one.