--- loncom/interface/lonparmset.pm	2016/07/12 20:30:20	1.560
+++ loncom/interface/lonparmset.pm	2016/07/15 18:03:52	1.561
@@ -1,7 +1,7 @@
 # The LearningOnline Network with CAPA
 # Handler to set parameters for assessments
 #
-# $Id: lonparmset.pm,v 1.560 2016/07/12 20:30:20 damieng Exp $
+# $Id: lonparmset.pm,v 1.561 2016/07/15 18:03:52 damieng Exp $
 #
 # Copyright Michigan State University Board of Trustees
 #
@@ -336,6 +336,10 @@ use LONCAPA qw(:DEFAULT :match);
 ##################################################
 
 # Page header
+#
+# @param {Apache2::RequestRec} $r - Apache request object
+# @param {string} $mode - selected tab, 'parmset' for course and problem settings, or 'coursepref' for course settings
+# @param {string} $crstype - course type ('Community' for community settings)
 sub startSettingsScreen {
     my ($r,$mode,$crstype)=@_;
 
@@ -367,18 +371,43 @@ sub endSettingsScreen {
 # (parmval is also used for the log of parameter changes)
 ##################################################
 
+# Calls parmval_by_symb, getting the symb from $id (the big hash resource id) with &symbcache.
+#
+# @param {string} $what - part info and parameter name separated by a dot, e.g. '0.weight'
+# @param {string} $id - big hash resource id
+# @param {string} $def - the resource's default value for this parameter
+# @param {string} $uname - user name
+# @param {string} $udom - user domain
+# @param {string} $csec - section name
+# @param {string} $cgroup - group name
+# @param {hash reference} $courseopt - course parameters hash (result of lonnet::get_courseresdata, dump of course's resourcedata.db)
+# @returns {Array}
 sub parmval {
     my ($what,$id,$def,$uname,$udom,$csec,$cgroup,$courseopt)=@_;
     return &parmval_by_symb($what,&symbcache($id),$def,$uname,$udom,$csec,
                                                            $cgroup,$courseopt);
 }
 
+# Returns an array containing
+# - the most specific level that is defined for that parameter (integer)
+# - an array with the level as index and the parameter value as value (when defined)
+#   (level 1 is the most specific and will have precedence)
+#
+# @param {string} $what - part info and parameter name separated by a dot, e.g. '0.weight'
+# @param {string} $symb - resource symb
+# @param {string} $def - the resource's default value for this parameter
+# @param {string} $uname - user name
+# @param {string} $udom - user domain
+# @param {string} $csec - section name
+# @param {string} $cgroup - group name
+# @param {hash reference} $courseopt - course parameters hash (result of lonnet::get_courseresdata, dump of course's resourcedata.db)
+# @returns {Array}
 sub parmval_by_symb {
     my ($what,$symb,$def,$uname,$udom,$csec,$cgroup,$courseopt)=@_;
 
     my $useropt;
     if ($uname ne '' && $udom ne '') {
-    $useropt = &Apache::lonnet::get_userresdata($uname,$udom);
+        $useropt = &Apache::lonnet::get_userresdata($uname,$udom);
     }
 
     my $result='';
@@ -386,7 +415,10 @@ sub parmval_by_symb {
 # ----------------------------------------------------- Cascading lookup scheme
     my $map=(&Apache::lonnet::decode_symb($symb))[0];
     $map = &Apache::lonnet::deversion($map);
-
+    
+    # NOTE: some of that code looks redondant with code in lonnavmaps::parmval_real,
+    # any change should be reflected there.
+    
     my $symbparm=$symb.'.'.$what;
     my $recurseparm=$map.'___(rec).'.$what; 
     my $mapparm=$map.'___(all).'.$what;
@@ -409,16 +441,19 @@ sub parmval_by_symb {
 
 # --------------------------------------------------------- first, check course
 
+# 18 - General Course
     if (defined($$courseopt{$courselevel})) {
         $outpar[18]=$$courseopt{$courselevel};
         $result=18;
     }
 
+# 17 - Map or Folder level in course (recursive) 
     if (defined($$courseopt{$courseleveli})) {
         $outpar[17]=$$courseopt{$courseleveli};
         $result=17;
     }
 
+# 16 - Map or Folder level in course (non-recursive)
     if (defined($$courseopt{$courselevelm})) {
         $outpar[16]=$$courseopt{$courselevelm};
         $result=16;
@@ -426,14 +461,17 @@ sub parmval_by_symb {
 
 # ------------------------------------------------------- second, check default
 
+# 15 - resource default
     if (defined($def)) { $outpar[15]=$def; $result=15; }
 
 # ------------------------------------------------------ third, check map parms
 
     
+# 14 - map default
     my $thisparm=&parmhash($symbparm);
     if (defined($thisparm)) { $outpar[14]=$thisparm; $result=14; }
 
+# 13 - resource level in course
     if (defined($$courseopt{$courselevelr})) {
         $outpar[13]=$$courseopt{$courselevelr};
         $result=13;
@@ -441,18 +479,22 @@ sub parmval_by_symb {
 
 # ------------------------------------------------------ fourth, back to course
     if ($csec ne '') {
+# 12 - General for section
         if (defined($$courseopt{$seclevel})) {
             $outpar[12]=$$courseopt{$seclevel};
             $result=12;
         }
+# 11 - Map or Folder level for section (recursive)
         if (defined($$courseopt{$secleveli})) {
             $outpar[11]=$$courseopt{$secleveli};
             $result=11;
         }
+# 10 - Map or Folder level for section (non-recursive)
         if (defined($$courseopt{$seclevelm})) {
             $outpar[10]=$$courseopt{$seclevelm};
             $result=10;
         }
+# 9 - resource level in section
         if (defined($$courseopt{$seclevelr})) {
             $outpar[9]=$$courseopt{$seclevelr};
             $result=9;
@@ -460,18 +502,22 @@ sub parmval_by_symb {
     }
 # ------------------------------------------------------ fifth, check course group
     if ($cgroup ne '') {
+# 8 - General for group
         if (defined($$courseopt{$grplevel})) {
             $outpar[8]=$$courseopt{$grplevel};
             $result=8;
         }
+# 7 - Map or Folder level for group (recursive)
         if (defined($$courseopt{$grpleveli})) {
             $outpar[7]=$$courseopt{$grpleveli};
             $result=7;
         }
+# 6 - Map or Folder level for group (non-recursive)
         if (defined($$courseopt{$grplevelm})) {
             $outpar[6]=$$courseopt{$grplevelm};
             $result=6;
         }
+# 5 - resource level in group
         if (defined($$courseopt{$grplevelr})) {
             $outpar[5]=$$courseopt{$grplevelr};
             $result=5;
@@ -481,25 +527,29 @@ sub parmval_by_symb {
 # ---------------------------------------------------------- sixth, check user
 
     if ($uname ne '') {
-    if (defined($$useropt{$courselevel})) {
-        $outpar[4]=$$useropt{$courselevel};
-        $result=4;
-    }
+# 4 - General for specific student
+        if (defined($$useropt{$courselevel})) {
+            $outpar[4]=$$useropt{$courselevel};
+            $result=4;
+        }
 
-    if (defined($$useropt{$courseleveli})) {
-        $outpar[3]=$$useropt{$courseleveli};
-        $result=3;
-    }
+# 3 - Map or Folder level for specific student (recursive)
+        if (defined($$useropt{$courseleveli})) {
+            $outpar[3]=$$useropt{$courseleveli};
+            $result=3;
+        }
 
-    if (defined($$useropt{$courselevelm})) {
-        $outpar[2]=$$useropt{$courselevelm};
-        $result=2;
-    }
+# 2 - Map or Folder level for specific student (non-recursive)
+        if (defined($$useropt{$courselevelm})) {
+            $outpar[2]=$$useropt{$courselevelm};
+            $result=2;
+        }
 
-    if (defined($$useropt{$courselevelr})) {
-        $outpar[1]=$$useropt{$courselevelr};
-        $result=1;
-    }
+# 1 - resource level for specific student
+        if (defined($$useropt{$courselevelr})) {
+            $outpar[1]=$$useropt{$courselevelr};
+            $result=1;
+        }
     }
     return ($result,@outpar);
 }
@@ -509,20 +559,25 @@ sub parmval_by_symb {
 # --- Caches local to lonparmset
 
 
+# Reset lonparmset caches (called at the beginning and end of the handler).
 sub reset_caches {
     &resetparmhash();
     &resetsymbcache();
     &resetrulescache();
 }
 
+# cache for map parameters, stored temporarily in $env{'request.course.fn'}_parms.db
+# (these parameters come from param elements in .sequence files created with the advanced RAT)
 {
-    my $parmhashid;
-    my %parmhash;
+    my $parmhashid; # course identifier, to initialize the cache only once for a course
+    my %parmhash; # the parameter cache
+    # reset map parameter hash
     sub resetparmhash {
         undef($parmhashid);
         undef(%parmhash);
     }
 
+    # dump the _parms.db database into %parmhash
     sub cacheparmhash {
         if ($parmhashid eq  $env{'request.course.fn'}) { return; }
         my %parmhashfile;
@@ -534,6 +589,7 @@ sub reset_caches {
         }
     }
 
+    # returns a parameter value for an identifier symb.parts.parameter, using the map parameter cache
     sub parmhash {
         my ($id) = @_;
         &cacheparmhash();
@@ -541,14 +597,17 @@ sub reset_caches {
     }
 }
 
+# cache big hash id -> symb, using lonnavmaps to find association
 {
-    my $symbsid;
-    my %symbs;
+    my $symbsid; # course identifier, to initialize the cache only once for a course
+    my %symbs; # hash id->symb
+    # reset the id->symb cache
     sub resetsymbcache {
         undef($symbsid);
         undef(%symbs);
     }
 
+    # returns the symb corresponding to a big hash id (using lonnavmaps and a cache)
     sub symbcache {
         my $id=shift;
         if ($symbsid ne $env{'request.course.id'}) {
@@ -569,14 +628,16 @@ sub reset_caches {
     }
 }
 
+# cache for parameter default actions (stored in parmdefactions.db)
 {
-    my $rulesid;
-    my %rules;
+    my $rulesid; # course identifier, to initialize the cache only once for a course
+    my %rules; # parameter default actions hash
     sub resetrulescache {
         undef($rulesid);
         undef(%rules);
     }
 
+    # returns the value for a given key in the parameter default action hash
     sub rulescache {
         my $id=shift;
         if ($rulesid ne $env{'request.course.id'}
@@ -591,7 +652,12 @@ sub reset_caches {
 }
 
 
-
+# Returns the values of the parameter type default action
+# "default value when manually setting".
+# If none is defined, ('','','','','') is returned.
+#
+# @param {string} $type - parameter type
+# @returns {Array<string>} - (hours, min, sec, value)
 sub preset_defaults {
     my $type=shift;
     if (&rulescache($type.'_action') eq 'default') {
@@ -607,8 +673,13 @@ sub preset_defaults {
 }
 
 
-
-
+# Checks that a date is after enrollment start date and before
+# enrollment end date.
+# Returns HTML with a warning if it is not, or the empty string otherwise.
+# This is used by both overview and table modes.
+#
+# @param {integer} $checkdate - the date to check.
+# @returns {string} - HTML possibly containing a localized warning message.
 sub date_sanity_info {
    my $checkdate=shift;
    unless ($checkdate) { return ''; }
@@ -643,26 +714,39 @@ sub date_sanity_info {
 #      }
    return $result;
 }
-##################################################
-##################################################
-#
-# Store a parameter by ID
-#
-# Takes
-# - resource id
-# - name of parameter
-# - level
-# - new value
-# - new type
-# - username
-# - userdomain
 
+
+# Store a parameter value and type by ID, also triggering more parameter changes based on parameter default actions.
+#
+# @param {string} $sresid - resource big hash id
+# @param {string} $spnam - part info and parameter name separated by a dot, e.g. '0.weight'
+# @param {integer} $snum - level
+# @param {string} $nval - new value
+# @param {string} $ntype - new type
+# @param {string} $uname - username
+# @param {string} $udom - userdomain
+# @param {string} $csec - section name
+# @param {string} $cgroup - group name
 sub storeparm {
     my ($sresid,$spnam,$snum,$nval,$ntype,$uname,$udom,$csec,$cgroup)=@_;
     &storeparm_by_symb(&symbcache($sresid),$spnam,$snum,$nval,$ntype,$uname,$udom,$csec,'',$cgroup);
 }
 
-my %recstack;
+my %recstack; # hash parameter name -> 1 when a parameter was used before in a recursive call to storeparm_by_symb
+
+# Store a parameter value and type by symb, also triggering more parameter changes based on parameter default actions.
+# Uses storeparm_by_symb_inner to actually store the parameter, ignoring any returned error.
+#
+# @param {string} $symb - resource symb
+# @param {string} $spnam - part info and parameter name separated by a dot, e.g. '0.weight'
+# @param {integer} $snum - level
+# @param {string} $nval - new value
+# @param {string} $ntype - new type
+# @param {string} $uname - username
+# @param {string} $udom - userdomain
+# @param {string} $csec - section name
+# @param {boolean} $recflag - should be true for recursive calls to storeparm_by_symb, false otherwise
+# @param {string} $cgroup - group name
 sub storeparm_by_symb {
     my ($symb,$spnam,$snum,$nval,$ntype,$uname,$udom,$csec,$recflag,$cgroup)=@_;
     unless ($recflag) {
@@ -715,10 +799,25 @@ sub storeparm_by_symb {
     return '';
 }
 
+# Adds all given arguments to the course parameter log.
+# @returns {string} - the answer to the lonnet query.
 sub log_parmset {
     return &Apache::lonnet::write_log('course','parameterlog',@_);
 }
 
+# Store a parameter value and type by symb, without using the parameter default actions.
+# Expire related sheets.
+#
+# @param {string} $symb - resource symb
+# @param {string} $spnam - part info and parameter name separated by a dot, e.g. '0.weight'
+# @param {integer} $snum - level
+# @param {string} $nval - new value
+# @param {string} $ntype - new type
+# @param {string} $uname - username
+# @param {string} $udom - userdomain
+# @param {string} $csec - section name
+# @param {string} $cgroup - group name
+# @returns {string} - HTML code with an error message if the parameter could not be stored.
 sub storeparm_by_symb_inner {
 # ---------------------------------------------------------- Get symb, map, etc
     my ($symb,$spnam,$snum,$nval,$ntype,$uname,$udom,$csec,$cgroup)=@_;
@@ -826,6 +925,16 @@ sub storeparm_by_symb_inner {
 }
 
 
+# Returns HTML with the value of the given parameter,
+# using a readable format for dates, and
+# a warning if there is a problem with a date.
+# Used by table mode.
+# Returns HTML for the editmap.png image if no value is defined and $editable is true.
+#
+# @param {string} $value - the parameter value
+# @param {string} $type - the parameter type
+# @param {string} $name - the parameter name (unused)
+# @param {boolean} $editable - Set to true to get an icon when no value is defined.
 sub valout {
     my ($value,$type,$name,$editable)=@_;
     my $result = '';
@@ -904,6 +1013,15 @@ sub valout {
 }
 
 
+# Returns HTML containing a link on a parameter value, for table mode.
+# The link uses the javascript function 'pjump'.
+#
+# @param {string} $type - parameter type
+# @param {string} $dis - dialog title for editing the parameter value and type
+# @param {string} $value - parameter value
+# @param {string} $marker - identifier for the parameter, "resource id&part_parameter name&level", will be passed as pres_marker when the user submits a change.
+# @param {string} $return - prefix for the name of the form and field names that will be used to submit the form ('parmform.pres')
+# @param {string} $call - javascript function to call to submit the form ('psub')
 sub plink {
     my ($type,$dis,$value,$marker,$return,$call)=@_;
     my $winvalue=$value;
@@ -930,6 +1048,7 @@ sub plink {
         $valout.'</a></td></tr></table>';
 }
 
+# Javascript for table mode.
 sub page_js {
 
     my $selscript=&Apache::loncommon::studentbrowser_javascript();
@@ -979,6 +1098,8 @@ ENDJS
 
 }
 
+# Javascript to show or hide the map selection (function showHide_courseContent),
+# for table and overview modes.
 sub showhide_js {
     return <<"COURSECONTENTSCRIPT";
 
@@ -999,6 +1120,7 @@ function showHide_courseContent() {
 COURSECONTENTSCRIPT
 }
 
+# Javascript functions showHideLenient and toggleParmTextbox, for overview mode
 sub toggleparmtextbox_js {
     return <<"ENDSCRIPT";
 
@@ -1058,6 +1180,7 @@ function toggleParmTextbox(form,key) {
 ENDSCRIPT
 }
 
+# Javascript function validateParms, for overview mode
 sub validateparms_js {
     return <<'ENDSCRIPT';
 
@@ -1131,6 +1254,7 @@ function validateParms() {
 ENDSCRIPT
 }
 
+# Javascript initialization, for overview mode
 sub ipacc_boxes_js  {
     my $remove = &mt('Remove');
     return <<"END";
@@ -1155,6 +1279,7 @@ sub ipacc_boxes_js  {
 END
 }
 
+# Javascript function toggleSecret, for overview mode.
 sub done_proctor_js {
     return <<"END";
 function toggleSecret(form,radio,key) {
@@ -1180,6 +1305,10 @@ END
 
 }
 
+# Prints HTML page start for table mode.
+# @param {Apache2::RequestRec} $r - the Apache request
+# @param {string} $psymb - resource symb
+# @param {string} $crstype - course type (Community / Course / Placement Test)
 sub startpage {
     my ($r,$psymb,$crstype) = @_;
 
@@ -1228,6 +1357,29 @@ ENDHEAD
 }
 
 
+# Prints a row for table mode (except for the tr start).
+# Every time a hash reference is passed, a single entry is used, so print_row
+# could just use these values, but why make it simple when it can be complicated ?
+#
+# @param {Apache2::RequestRec} $r - the Apache request
+# @param {string} $which - parameter key ('parameter_'.part.'_'.name)
+# @param {hash reference} $part - parameter key -> parameter part (can be problem part.'_'.response id for response parameters)
+# @param {hash reference} $name - parameter key -> parameter name
+# @param {hash reference} $symbp - resource id -> symb
+# @param {string} $rid - resource id
+# @param {hash reference} $default - parameter key -> resource parameter default value
+# @param {hash reference} $defaulttype - parameter key -> resource parameter default type
+# @param {hash reference} $display - parameter key -> full title for the parameter
+# @param {string} $defbgone - user level and other levels background color
+# @param {string} $defbgtwo - section level background color, also used for part number
+# @param {string} $defbgthree - group level background color
+# @param {string} $parmlev - parameter level (Resource:'full', Map:'map', Course:'general')
+# @param {string} $uname - user name
+# @param {string} $udom - user domain
+# @param {string} $csec - section name
+# @param {string} $cgroup - group name
+# @param {array reference} $usersgroups - list of groups the user belongs to, if any
+# @param {boolean} $noeditgrp - true if no edit is allowed for group level parameters
 sub print_row {
     my ($r,$which,$part,$name,$symbp,$rid,$default,$defaulttype,$display,$defbgone,
     $defbgtwo,$defbgthree,$parmlev,$uname,$udom,$csec,$cgroup,$usersgroups,$noeditgrp)=@_;
@@ -1364,6 +1516,22 @@ sub print_row {
     $r->print("\n");
 }
 
+# Prints a cell for table mode.
+#
+# FIXME: some of these parameter names are uninspired ($which and $value)
+# Also, it would make more sense to pass the display for this cell rather
+# than the full display hash and the key to use.
+#
+# @param {Apache2::RequestRec} $r - the Apache request
+# @param {integer} $which - level
+# @param {string} $defbg - cell background color
+# @param {integer} $result - the most specific level that is defined for that parameter
+# @param {array reference} $outpar - array level -> parameter value (when defined)
+# @param {string} $mprefix - resource id.'&'.part.'_'.parameter name.'&'
+# @param {string} $value - parameter key ('parameter_'.part.'_'.name)
+# @param {array reference} $typeoutpar - array level -> parameter type (when defined)
+# @param {hash reference} $display - parameter key -> full title for the parameter
+# @param {boolean} $noeditgrp - true if no edit is allowed for group level parameters
 sub print_td {
     my ($r,$which,$defbg,$result,$outpar,$mprefix,$value,$typeoutpar,$display,$noeditgrp)=@_;
     $r->print('<td style="background-color:'.(($result==$which)?'#AAFFAA':$defbg).
@@ -1388,6 +1556,7 @@ sub print_td {
     }
     if ($nolink) {
         $r->print(&valout($$outpar[$which],$$typeoutpar[$which],$mprefix));
+# FIXME: probably a good thing that mprefix is not used in valout, because it does not look like a parameter name !
     } else {
         $r->print(&plink($$typeoutpar[$which],
                          $$display{$value},$$outpar[$which],
@@ -1396,6 +1565,19 @@ sub print_td {
     $r->print('</td>'."\n");
 }
 
+# FIXME: Despite the name, this does not print anything, the $r parameter is unused.
+# Returns HTML and other info for the cell added when a user is selected
+# and that user is in several groups. This is the cell with the title "Control by other group".
+#
+# @param {Apache2::RequestRec} $r - the Apache request (unused)
+# @param {string} $what - parameter part.'.'.parameter name
+# @param {string} $rid - resource id
+# @param {string} $cgroup - group name
+# @param {string} $defbg - cell background color
+# @param {array reference} $usersgroups - list of groups the user belongs to, if any
+# @param {integer} $result - level
+# @param {hash reference} $courseopt - course parameters hash (result of lonnet::get_courseresdata, dump of course's resourcedata.db)
+# @returns {Array} - array (parameter value for the other group, HTML for the cell, HTML with the value, name of the other group)
 sub print_usergroups {
     my ($r,$what,$rid,$cgroup,$defbg,$usersgroups,$result,$courseopt) = @_;
     my $courseid = $env{'request.course.id'};
@@ -1429,6 +1611,17 @@ sub print_usergroups {
     return ($coursereply,$output,$grp_parm,$resultgroup);
 }
 
+# Looks for a group with a defined parameter for given user and parameter.
+# Used by print_usergroups.
+#
+# @param {string} $courseid - the course id
+# @param {array reference} $usersgroups - list of groups the user belongs to, if any
+# @param {string} $symbparm - end of the course parameter hash key for the group resource level
+# @param {string} $mapparm - end of the course parameter hash key for the group map/folder level
+# @param {string} $recurseparm - end of the course parameter hash key for the group recursive level
+# @param {string} $what - parameter part.'.'.parameter name
+# @param {hash reference} $courseopt - course parameters hash
+# @returns {Array} - (parameter value for the group, course parameter hash key for the parameter, name of the group, level name, parameter type)
 sub parm_control_group {
     my ($courseid,$usersgroups,$symbparm,$mapparm,$recurseparm,$what,$courseopt) = @_;
     my ($coursereply,$resultitem,$resultgroup,$resultlevel,$resulttype);
@@ -1455,6 +1648,7 @@ sub parm_control_group {
 
 
 
+# extracts lots of information about all of the the course's resources into a variety of hashes.
 sub extractResourceInformation {
     my $ids = shift;
     my $typep = shift;