Jeroen T. Vermeulen has proposed merging 
lp:~jtv/launchpad/bug-662552-view-permissions into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers): code
Related bugs:
  #662552 Timeout error trying to access the translation interface
  https://bugs.launchpad.net/bugs/662552


= 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
-- 
https://code.launchpad.net/~jtv/launchpad/bug-662552-view-permissions/+merge/39481
Your team Launchpad code reviewers is requested to review the proposed merge of 
lp:~jtv/launchpad/bug-662552-view-permissions into lp:launchpad/devel.
=== modified file 'lib/lp/translations/browser/pofile.py'
--- lib/lp/translations/browser/pofile.py	2010-08-31 11:11:09 +0000
+++ lib/lp/translations/browser/pofile.py	2010-10-27 21:14:47 +0000
@@ -854,9 +854,12 @@
                         self.context.potemplate, self.context.language))
             else:
                 translationmessage.setPOFile(self.context)
+
+            error = self.errors.get(potmsgset)
+            can_edit = self.context.canEditTranslations(self.user)
             view = self._prepareView(
                 CurrentTranslationMessageView, translationmessage,
-                self.errors.get(potmsgset))
+                pofile=self.context, can_edit=can_edit, error=error)
             view.zoomed_in_view = False
             self.translationmessage_views.append(view)
 

=== modified file 'lib/lp/translations/browser/translationmessage.py'
--- lib/lp/translations/browser/translationmessage.py	2010-09-03 16:01:01 +0000
+++ lib/lp/translations/browser/translationmessage.py	2010-10-27 21:14:47 +0000
@@ -463,8 +463,9 @@
                 return False
         return True
 
-    def _prepareView(self, view_class, current_translation_message, error):
-        """Collect data and build a TranslationMessageView for display."""
+    def _prepareView(self, view_class, current_translation_message,
+                     pofile, can_edit, error=None):
+        """Collect data and build a `TranslationMessageView` for display."""
         # XXX: kiko 2006-09-27:
         # It would be nice if we could easily check if this is being
         # called in the right order, after _storeTranslations().
@@ -509,7 +510,7 @@
             current_translation_message, self.request,
             plural_indices_to_store, translations, force_suggestion,
             force_diverge, error, self.second_lang_code,
-            self.form_is_writeable)
+            self.form_is_writeable, pofile=pofile, can_edit=can_edit)
 
     #
     # Internals
@@ -838,8 +839,11 @@
 
     def _initializeTranslationMessageViews(self):
         """See `BaseTranslationView._initializeTranslationMessageViews`."""
+        pofile = self.pofile
+        can_edit = pofile.canEditTranslations(self.user)
         self.translationmessage_view = self._prepareView(
-            CurrentTranslationMessageZoomedView, self.context, self.error)
+            CurrentTranslationMessageZoomedView, self.context, pofile=pofile,
+            can_edit=can_edit, error=self.error)
 
     def _submitTranslations(self):
         """See `BaseTranslationView._submitTranslations`."""
@@ -900,7 +904,8 @@
 
     def __init__(self, current_translation_message, request,
                  plural_indices_to_store, translations, force_suggestion,
-                 force_diverge, error, second_lang_code, form_is_writeable):
+                 force_diverge, error, second_lang_code, form_is_writeable,
+                 pofile, can_edit):
         """Primes the view with information that is gathered by a parent view.
 
         :param plural_indices_to_store: A dictionary that indicates whether
@@ -916,17 +921,18 @@
             field.alternative_value.
         :param form_is_writeable: Whether the form should accept write
             operations
+        :param pofile: The `POFile` that's being displayed or edited.
+        :param can_edit: Whether the user has editing privileges on `pofile`.
         """
         LaunchpadView.__init__(self, current_translation_message, request)
 
-        self.pofile = self.context.browser_pofile
+        self.pofile = pofile
         self.plural_indices_to_store = plural_indices_to_store
         self.translations = translations
         self.error = error
         self.force_suggestion = force_suggestion
         self.force_diverge = force_diverge
-        self.user_is_official_translator = (
-            self.pofile.canEditTranslations(self.user))
+        self.user_is_official_translator = can_edit
         self.form_is_writeable = form_is_writeable
         if self.context.is_imported:
             # The imported translation matches the current one.
@@ -1053,7 +1059,7 @@
 
             diverged_and_have_shared = (
                 self.context.potemplate is not None and
-                self.shared_translationmessage is not None) 
+                self.shared_translationmessage is not None)
             if diverged_and_have_shared:
                 pofile = self.shared_translationmessage.ensureBrowserPOFile()
                 if pofile is None:
@@ -1152,10 +1158,10 @@
 
     def _setOnePOFile(self, messages):
         """Return a list of messages that all have a browser_pofile set.
-        
+
         If a pofile cannot be found for a message, it is not included in
         the resulting list.
-        """ 
+        """
         result = []
         for message in messages:
             if message.browser_pofile is None:
@@ -1167,7 +1173,7 @@
                     message.setPOFile(pofile)
             result.append(message)
         return result
-    
+
     def _buildAllSuggestions(self):
         """Builds all suggestions and puts them into suggestions_block.
 

_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help   : https://help.launchpad.net/ListHelp

Reply via email to