Added comments to change after Vince code review

1 files changed, 35 insertions, 26 deletions

diff --git a/tutorius/vault.py b/tutorius/vault.py
index 6c40f26..2429813 100644
--- a/tutorius/vault.py
+++ b/tutorius/vault.py
@@ -29,6 +29,7 @@ import zipfile
 
 from sugar.tutorius import addon
 from sugar.tutorius.core import Tutorial, State, FiniteStateMachine
+# TODO : Import only revelent
 from sugar.tutorius.filters import *
 from sugar.tutorius.actions import *
 from ConfigParser import *
@@ -42,6 +43,9 @@ def _get_store_root():
                         ".sugar",profile_name,"tutorius","data")
 # this is where activity bundled tutorials should be, under the activity bundle
 def _get_bundle_root():
+    """
+    Return the path of the bundled activity, or None if not applicable.
+    """
     if os.getenv("SUGAR_BUNDLE_PATH") != None:
         return os.path.join(os.getenv("SUGAR_BUNDLE_PATH"),"data","tutorius","data")
     else:
@@ -60,32 +64,34 @@ NODE_COMPONENT = "Component"
 class Vault(object):
     
     ## Vault internal functions :    
-        
+    # TODO : Make vault static : add @staticmethod over and remove self from argument
     def list_available_tutorials(self, activity_name = 'all', activity_vers = 0):
+        # TODO : remove extreme case if activity name is 'all'
         """
         Generate the list of all tutorials present on disk for a
         given activity.
 
         @param activity_name the name of the activity associated with this tutorial. 'all' means ALL activities
-        @param activity_vers the version number of the activity to find tutorail for. 0 means find for ANY version
+        @param activity_vers the version number of the activity to find tutorail for. 0 means find for ANY version. Ifactivity_ame is 'all', version number is not used
         @returns a map of tutorial {names : GUID}.
         """
         # check both under the activity data and user installed folders
         if _get_bundle_root() != None:
             paths = [_get_store_root(), _get_bundle_root()]
         else:
-            paths[_get_store_root]
+            paths = [_get_store_root()]
 
         tutoGuidName = {}
 
         for repository in paths:
             # (our) convention dictates that tutorial folders are named
-            # with their GUID (for unicity) but this is not enforced.
+            # with their GUID (for unicity)
             try:
                 for tuto in os.listdir(repository):
                     parser = SafeConfigParser()
                     file = parser.read(os.path.join(repository, tuto, INI_FILENAME))
                     if file != []:
+                        # If parser can read at least section
                         guid = parser.get(INI_METADATA_SECTION, INI_GUID_PROPERTY)
                         name = parser.get(INI_METADATA_SECTION, INI_NAME_PROPERTY)
                         activities = parser.options(INI_ACTIVITY_SECTION)
@@ -122,17 +128,18 @@ class Vault(object):
         @returns 0 if it worked, 1 if the user needs to confirm the installation
         and 2 to mean an error happened
         """
+        # TODO : Check with architecture team for exception vs error returns
         
         # test if the file is a valid pkzip file
         if zipfile.is_zipfile(os.path.join(path, zip_file_name)) != True:
             assert False, "Error : The given file is not a valid PKZip file"
-            
+                        
         # unpack the zip archive
         zfile = zipfile.ZipFile(os.path.join(path, zip_file_name), "r" )
         
         temp_path = os.path.join(_get_store_root(),'temp_extracted_tuto')
         os.makedirs(temp_path)
-        
+        # TODO : use extractall for extracting instead to reduce code base
         for info in zfile.infolist():
             fname = info.filename
             # decompress each file's data
@@ -286,12 +293,13 @@ class Vault(object):
             parser = SafeConfigParser()
             parser.add_section(INI_METADATA_SECTION)
             for key_value in metadata_dict.items():
+                # TODO : change for key, value
                 if key_value[0] != 'activities':
                     parser.set(INI_METADATA_SECTION, key_value[0], key_value[1])
                 else:
                     related_activities_dict = key_value[1]
                     parser.add_section(INI_ACTIVITY_SECTION)
-                    for related_key_value in related_activities_dict:
+                    for related_key_value in related_activities_dict.items():
                         parser.set(INI_ACTIVITY_SECTION, related_key_value[0], related_key_value[1])
 
             # Write the file to disk
@@ -302,23 +310,23 @@ class Vault(object):
             # Error, tutorial already exist
             return False
         
-        
-    def deleteTutorial(Tutorial):
-        """
-        Removes the tutorial from the Vault. It will unpublish the tutorial if need be,
-        and it will also wipe it from the persistent storage.
-        @returns true is the tutorial was deleted from the Vault
-        """
-        bundle = TutorialBundler(Guid)
-        bundle_path = bundle.get_tutorial_path(Guid)
-        
-        # TODO : Need also to unpublish tutorial, need to interact with webservice module
-        
-        shutil.rmtree(bundle_path)
-        if os.path.isdir(bundle_path) == False:
-            return True
-        else:
-            return False
+    # TODO : wait for Ben input on how to unpublish tuto before coding this function
+##    def deleteTutorial(Tutorial):
+##        """
+##        Removes the tutorial from the Vault. It will unpublish the tutorial if need be,
+##        and it will also wipe it from the persistent storage.
+##        @returns true is the tutorial was deleted from the Vault
+##        """
+##        bundle = TutorialBundler(Guid)
+##        bundle_path = bundle.get_tutorial_path(Guid)
+##        
+##        # TODO : Need also to unpublish tutorial, need to interact with webservice module
+##        
+##        shutil.rmtree(bundle_path)
+##        if os.path.isdir(bundle_path) == False:
+##            return True
+##        else:
+##            return False
         
 
 class Serializer(object):
@@ -335,13 +343,13 @@ class Serializer(object):
         exception occur. If no GUID is provided, FSM is written in a new file 
         in the store root.
         """
-        NotImplementedError
+        raise NotImplementedError()
         
     def load_fsm(self):
         """
         Load fsm from disk.
         """
-        NotImplementedError
+        raise NotImplementedError()
         
 class XMLSerializer(Serializer):
     """
@@ -635,6 +643,7 @@ class TutorialBundler(object):
         cfg.set(INI_METADATA_SECTION, INI_NAME_PROPERTY, tutorial.name)
         cfg.set(INI_METADATA_SECTION, INI_XML_FSM_PROPERTY, TUTORIAL_FILENAME)
         cfg.add_section(INI_ACTIVITY_SECTION)
+        # TODO : Check behavior if environment varaibles dont exist
         cfg.set(INI_ACTIVITY_SECTION, os.environ['SUGAR_BUNDLE_NAME'],
                 os.environ['SUGAR_BUNDLE_VERSION'])