--- trunk/webwork2/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetDetail.pm	2004/10/21 01:44:14	2948
+++ trunk/webwork2/lib/WeBWorK/ContentGenerator/Instructor/ProblemSetDetail.pm	2010/05/31 15:35:33	6287
@@ -1,6 +1,6 @@
 ################################################################################
 # WeBWorK Online Homework Delivery System
-# Copyright © 2000-2003 The WeBWorK Project, http://openwebwork.sf.net/
+# Copyright © 2000-2007 The WeBWorK Project, http://openwebwork.sf.net/
 # 
 # 
 # This program is free software; you can redistribute it and/or modify it under
@@ -25,25 +25,40 @@
 
 use strict;
 use warnings;
-use CGI qw();
+#use CGI qw(-nosticky );
+use WeBWorK::CGI;
 use WeBWorK::HTML::ComboBox qw/comboBox/;
-use WeBWorK::Utils qw(readDirectory list2hash listFilesRecursive max);
-use WeBWorK::DB::Record::Set;
+use WeBWorK::Utils qw(readDirectory list2hash listFilesRecursive max cryptPassword);
 use WeBWorK::Utils::Tasks qw(renderProblems);
 use WeBWorK::Debug;
+# IP RESTRICT
+use WeBWorK::HTML::ScrollingRecordList qw/scrollingRecordList/;
 
 # Important Note: the following two sets of constants may seem similar 
 # 	but they are functionally and semantically different
 
 # these constants determine which fields belong to what type of record
-use constant SET_FIELDS => [qw(set_header hardcopy_header open_date due_date answer_date published)];
+use constant SET_FIELDS => [qw(set_header hardcopy_header open_date due_date answer_date published enable_reduced_scoring restrict_ip relax_restrict_ip assignment_type attempts_per_version version_time_limit time_limit_cap versions_per_interval time_interval problem_randorder problems_per_page hide_score:hide_score_by_problem hide_work)];
 use constant PROBLEM_FIELDS =>[qw(source_file value max_attempts)];
 use constant USER_PROBLEM_FIELDS => [qw(problem_seed status num_correct num_incorrect)];
 
 # these constants determine what order those fields should be displayed in
 use constant HEADER_ORDER => [qw(set_header hardcopy_header)];
 use constant PROBLEM_FIELD_ORDER => [qw(problem_seed status value max_attempts attempted last_answer num_correct num_incorrect)];
-use constant SET_FIELD_ORDER => [qw(open_date due_date answer_date published)];
+# for gateway sets, we don't want to allow users to change max_attempts on a per
+#    problem basis, as that's nothing but confusing.
+use constant GATEWAY_PROBLEM_FIELD_ORDER => [qw(problem_seed status value attempted last_answer num_correct num_incorrect)];
+
+# we exclude the gateway set fields from the set field order, because they
+#     are only displayed for sets that are gateways.  this results in a bit of
+#     convoluted logic below, but it saves burdening people who are only using
+#     homework assignments with all of the gateway parameters
+# FIXME: in the long run, we may want to let hide_score and hide_work be
+# FIXME: set for non-gateway assignments.  right now (11/30/06) they are
+# FIXME: only used for gateways
+use constant SET_FIELD_ORDER => [qw(open_date due_date answer_date published enable_reduced_scoring restrict_ip relax_restrict_ip assignment_type)];
+# use constant GATEWAY_SET_FIELD_ORDER => [qw(attempts_per_version version_time_limit time_interval versions_per_interval problem_randorder problems_per_page hide_score hide_work)];
+use constant GATEWAY_SET_FIELD_ORDER => [qw(version_time_limit time_limit_cap attempts_per_version time_interval versions_per_interval problem_randorder problems_per_page hide_score:hide_score_by_problem hide_work)];
 
 # this constant is massive hash of information corresponding to each db field.
 # override indicates for how many students at a time a field can be overridden
@@ -61,6 +76,10 @@
 #				1 => "Yes",
 #				0 => "No",
 #		},
+#               convertby => 60,                # divide incoming database field values by this, and multiply when saving
+
+use constant BLANKPROBLEM => 'blankProblem.pg';
+
 use constant  FIELD_PROPERTIES => {
 	# Set information
 	set_header => {
@@ -85,7 +104,7 @@
 		size      => "26",
 		override  => "any",
 		labels    => {
-				0 => "None Specified",
+				#0 => "None Specified",
 				"" => "None Specified",
 		},
 	},
@@ -95,7 +114,7 @@
 		size      => "26",
 		override  => "any",
 		labels    => {
-				0 => "None Specified",
+				#0 => "None Specified",
 				"" => "None Specified",
 		},
 	},
@@ -105,7 +124,7 @@
 		size      => "26",
 		override  => "any",
 		labels    => {
-				0 => "None Specified",
+				#0 => "None Specified",
 				"" => "None Specified",
 		},
 	},
@@ -119,6 +138,125 @@
 				0 => "No",
 		},
 	},
+	enable_reduced_scoring => {
+		name      => "Reduced Credit Enabled",
+		type      => "choose",
+		override  => "all",
+		choices   => [qw( 0 1 )],
+		labels    => {
+				1 => "Yes",
+				0 => "No",
+		},
+	},
+	restrict_ip => {
+		name      => "Restrict Access by IP",
+		type      => "choose",
+		override  => "any",
+		choices   => [qw( No RestrictTo DenyFrom )],
+		labels    => {
+				No => "No",
+				RestrictTo => "Restrict To",
+				DenyFrom => "Deny From",
+		},
+		default   => 'No',
+	},
+	relax_restrict_ip => { 
+		name      => "Relax IP restrictions when?",
+		type      => "choose",
+		override  => "any",
+		choices   => [qw( No AfterAnswerDate AfterVersionAnswerDate )],
+		labels    => {
+				No => "Never",
+				AfterAnswerDate => "After set answer date",
+				AfterVersionAnswerDate => "(gw/quiz) After version answer date",
+		},
+		default   => 'No',
+	},
+	assignment_type => {
+		name      => "Assignment type",
+		type      => "choose",
+		override  => "all",
+		choices   => [qw( default gateway proctored_gateway )],
+		labels    => {	default => "homework",
+				gateway => "gateway/quiz",
+				proctored_gateway => "proctored gateway/quiz",
+		},
+	},
+	version_time_limit => {
+		name      => "Test Time Limit (min)",
+		type      => "edit",
+		size      => "4",
+		override  => "any",
+#		labels    => {	"" => 0 },  # I'm not sure this is quite right
+		convertby => 60,
+	},
+	time_limit_cap => {
+		name      => "Cap Test Time at Set Due Date?",
+		type      => "choose",
+		override  => "all",
+		choices   => [qw(0 1)],
+		labels    => { '0' => 'No', '1' => 'Yes' },
+	},
+	attempts_per_version => {
+		name      => "Number of Graded Submissions per Test",
+		type      => "edit",
+		size      => "3",
+		override  => "any",
+#		labels    => {	"" => 1 },
+	},
+	time_interval => {
+		name      => "Time Interval for New Test Versions (min; 0=infty)",
+		type      => "edit",
+                size      => "5",
+		override  => "any",
+#		labels    => {	"" => 0 },
+		convertby => 60,
+	},
+	versions_per_interval => {
+		name      => "Number of Tests per Time Interval (0=infty)",
+		type      => "edit",
+                size      => "3",
+		override  => "any",
+		default   => "0",
+		format    => '[0-9]+',      # an integer, possibly zero
+#		labels    => {	"" => 0 },
+#		labels    => {	"" => 1 },
+	},
+	problem_randorder => {
+		name      => "Order Problems Randomly",
+		type      => "choose",
+		choices   => [qw( 0 1 )],
+		override  => "any",
+		labels    => {	0 => "No", 1 => "Yes" },
+	},
+	problems_per_page => {
+	        name      => "Number of Problems per Page (0=all)",
+		type      => "edit",
+		size      => "3",
+		override  => "any",
+		default   => "0",
+#		labels    => { "" => 0 },
+	},
+	'hide_score:hide_score_by_problem' => {
+		name      => "Show Scores on Finished Assignments?",
+		type      => "choose",
+		choices   => [ qw( N: Y:N BeforeAnswerDate:N Y:Y BeforeAnswerDate:Y ) ],
+		override  => "any",
+		labels    => { 'N:' => 'Yes', 'Y:N' => 'No', 'BeforeAnswerDate:N' => 'Only after set answer date', 'Y:Y' => 'Totals only (not problem scores)', 'BeforeAnswerDate:Y' => 'Totals only, only after answer date' },
+	},
+	hide_work         => {
+		name      => "Show Student Work on Finished Tests",
+		type      => "choose",
+		choices   => [ qw(N Y BeforeAnswerDate) ],
+		override  => "any",
+		labels    => { 'N' => "Yes", 'Y' => "No", 'BeforeAnswerDate' => 'Only after set answer date' },
+	},
+	# in addition to the set fields above, there are a number of things
+	#    that are set but aren't in this table:
+	#    any set proctor information (which is in the user tables), and
+	#    any set location restriction information (which is in the 
+	#    location tables)
+	#
 	# Problem information
 	source_file => {
 		name      => "Source File",
@@ -190,7 +328,7 @@
 # if only the setID is included, it creates a table of set information
 # if the problemID is included, it creates a table of problem information
 sub FieldTable {
-	my ($self, $userID, $setID, $problemID, $globalRecord, $userRecord) = @_;
+	my ($self, $userID, $setID, $problemID, $globalRecord, $userRecord, $isGWset) = @_;
 
 	my $r = $self->r;	
 	my @editForUser = $r->param('editForUser');
@@ -198,24 +336,70 @@
 	my $forOneUser  = $forUsers == 1;
 
 	my @fieldOrder;
+
+	# needed for gateway output
+	my $gwFields = '';
+	# $isGWset will come in undef if we don't need to worry about it
+	$isGWset = 0 if ( ! defined( $isGWset ) );
+	# are we editing a set version?
+	my $setVersion = (defined($userRecord) && $userRecord->can("version_id")) ? 1 : 0;
+
+	# needed for ip restrictions
+	my $ipFields = '';
+	my $ipDefaults;
+	my $numLocations = 0;
+	my $ipOverride;
+
+	# needed for set-level proctor
+	my $procFields = '';
+
 	if (defined $problemID) {
-		@fieldOrder = @{ PROBLEM_FIELD_ORDER() };
+		@fieldOrder = ($isGWset) ? @{ GATEWAY_PROBLEM_FIELD_ORDER() } :
+			@{ PROBLEM_FIELD_ORDER() };
 	} else {
 		@fieldOrder = @{ SET_FIELD_ORDER() };
+
+		($gwFields, $ipFields, $numLocations, $procFields) = $self->extraSetFields($userID, $setID, $globalRecord, $userRecord, $forUsers);
 	}
 
 	my $output = CGI::start_table({border => 0, cellpadding => 1});
 	if ($forUsers) {
-		$output .= CGI::Tr(
-			CGI::th({colspan=>"3"}, "User Value"),
-			CGI::th({}, "Global Value"),
+		$output .= CGI::Tr({},
+		    CGI::th({colspan=>"2"}, " "),
+			CGI::th({colspan=>"1"}, "User Values"),
+			CGI::th({}, "Class values"),
 		);
 	}
-	
 	foreach my $field (@fieldOrder) {
 		my %properties = %{ FIELD_PROPERTIES()->{$field} };
+
+		# we don't show the ip restriction option if there are 
+		#    no defined locations, nor the relax_restrict_ip option
+		#    if we're not restricting ip access
+		next if ( $field eq 'restrict_ip' && ( ! $numLocations || $setVersion ) );
+		next if ($field eq 'relax_restrict_ip' &&
+			 (! $numLocations || $setVersion ||
+			  ($forUsers && $userRecord->restrict_ip eq 'No') ||
+			  (! $forUsers && 
+			   ( $globalRecord->restrict_ip eq '' ||
+			     $globalRecord->restrict_ip eq 'No' ) ) ) );
+		# skip the problem seed if we're editing a gateway set for users,
+		#    but aren't editing a set version
+		next if ( $field eq 'problem_seed'  &&
+			  ( $isGWset && $forUsers && ! $setVersion ) );
+
 		unless ($properties{type} eq "hidden") {
-			$output .= CGI::Tr({}, CGI::td({}, [$self->FieldHTML($userID, $setID, $problemID, $globalRecord, $userRecord, $field)]));
+			$output .= CGI::Tr({}, CGI::td({}, [$self->FieldHTML($userID, $setID, $problemID, $globalRecord, $userRecord, $field)])) . "\n";
+	}
+
+		# finally, put in extra fields that are exceptions to the 
+		#    usual display mechanism
+		if ( $field eq 'restrict_ip' && $ipFields ) {
+			$output .= $ipFields;
+		}
+					      
+		if ( $field eq 'assignment_type' ) {
+			$output .= "$procFields\n$gwFields\n";
 		}
 	} 
 
@@ -266,14 +450,48 @@
 	my $edit = ($properties{type} eq "edit") && ($properties{override} ne "none");
 	my $choose = ($properties{type} eq "choose") && ($properties{override} ne "none");
 	
-	my $globalValue = $globalRecord->{$field};
-	$globalValue = $globalValue ? ($labels{$globalValue || ""} || $globalValue) : "";
-	my $userValue = $userRecord->{$field};
-	$userValue = $userValue ? ($labels{$userValue || ""} || $userValue) : "";
+# FIXME: allow one selector to set multiple fields
+#	my $globalValue = $globalRecord->{$field};
+# 	my $userValue = $userRecord->{$field};
+	my ($globalValue, $userValue) = ('', '');
+	my $blankfield = '';
+	if ( $field =~ /:/ ) {
+		my @gVals = ();
+		my @uVals = ();
+		my @bVals = ();
+		foreach my $f ( split(/:/, $field) ) {
+			# hmm.  this directly references the data in the 
+			#    record rather than calling the access method, 
+			#    thereby avoiding errors if the userRecord is 
+			#    undefined.  that seems a bit suspect, but it's
+			#    used below so we'll leave it here.
+
+			push(@gVals, $globalRecord->{$f} );
+			push(@uVals, $userRecord->{$f} );    # (defined($userRecord->{$f})?$userRecord->{$f}:'') );
+			push(@bVals, '');
+		}
+		# I don't like this, but combining multiple values is a bit messy
+		$globalValue = (grep {defined($_)} @gVals) ? join(':', (map { defined($_) ? $_ : '' } @gVals )) : undef;
+		$userValue = (grep {defined($_)} @uVals) ? join(':', (map { defined($_) ? $_ : '' } @uVals )) : undef;
+		$blankfield = join(':', @bVals);
+	} else {
+		$globalValue = $globalRecord->{$field};
+		$userValue = $userRecord->{$field};
+	}
+
+	# use defined instead of value in order to allow 0 to printed, e.g. for the 'value' field
+	$globalValue = (defined($globalValue)) ? ($labels{$globalValue || ""} || $globalValue) : "";
+	$userValue = (defined($userValue)) ? ($labels{$userValue || ""} || $userValue) : $blankfield;
 
 	if ($field =~ /_date/) {
-		$globalValue = $self->formatDateTime($globalValue) if $globalValue;
-		$userValue = $self->formatDateTime($userValue) if $userValue;
+		$globalValue = $self->formatDateTime($globalValue) if defined $globalValue && $globalValue ne $labels{""};
+		# this is still fragile, but the check for blank (as opposed to 0) $userValue seems to prevent errors when no user has been assigned.
+		$userValue = $self->formatDateTime($userValue) if defined $userValue && $userValue =~/\S/ && $userValue ne $labels{""};
+	}
+
+	if ( defined($properties{convertby}) && $properties{convertby} ) {
+		$globalValue = $globalValue/$properties{convertby} if $globalValue;
+		$userValue = $userValue/$properties{convertby} if $userValue;
 	}
 
 	# check to make sure that a given value can be overridden
@@ -304,27 +522,185 @@
 		# Note that in popup menus, you're almost guaranteed to have the choices hashed to labels in %properties
 		# but $userValue and and $globalValue are the values in the hash not the keys
 		# so we have to use the actual db record field values to select our default here.
+
+		# FIXME: this allows us to set one selector from two (or more) fields
+		# if $field matches /:/, we have to get two fields to get the data we need here
+		my $value = $r->param("$recordType.$recordID.$field");
+		if ( ! $value && $field =~ /:/ ) { 
+			my @fields = split(/:/, $field);
+			$value = '';
+			foreach my $f ( @fields ) {
+				$value .= ($forUsers && $userRecord->$f ne '' ? $userRecord->$f : $globalRecord->$f) . ":";
+			}
+			$value =~ s/:$//;
+		} elsif ( ! $value ) {
+			$value = ($forUsers && $userRecord->$field ne '' ? $userRecord->$field : $globalRecord->$field);
+		}
+			
 		$inputType = CGI::popup_menu({
 				name => "$recordType.$recordID.$field",
 				values => $properties{choices},
 				labels => \%labels,
-				default => $r->param("$recordType.$recordID.$field") || ($forUsers ? $userRecord->$field : $globalRecord->$field),
+				default => $value,
 		});
 	}
 	
-	return (($forUsers && $edit && $check) ? CGI::checkbox({
+	my $gDisplVal = defined($properties{labels}) && defined($properties{labels}->{$globalValue}) ? $properties{labels}->{$globalValue} : $globalValue;
+
+	# FIXME: adding ":" in the checked => allows for multiple fields to be set by one selector
+#	return (($forUsers && $edit && $check) ? CGI::checkbox({
+	return (($forUsers && $check) ? CGI::checkbox({
 				type => "checkbox",
 				name => "$recordType.$recordID.$field.override",
 				label => "",
 				value => $field,
-				checked => $r->param("$recordType.$recordID.$field.override") || ($userValue ne "" ? 1 : 0),
+				checked => $r->param("$recordType.$recordID.$field.override") || ($userValue ne ($labels{""} || $blankfield) ? 1 : 0),
 		}) : "",
 		$properties{name},
 		$inputType,
-		$forUsers ? " $globalValue" : "",
+		$forUsers ? " $gDisplVal" : "",
 	);
 }
 
+# return weird fields that are non-native or which are displayed
+#    for only some sets
+sub extraSetFields {
+	my ($self,$userID,$setID,$globalRecord,$userRecord,$forUsers) = @_;
+	my $db = $self->r->{db};
+
+	my ($gwFields, $ipFields, $ipDefaults, $numLocations, $ipOverride,
+	    $procFields) = ( '', '', '', 0, '', '' );
+
+	# if we're dealing with a gateway, set up a table of gateway fields
+	my $nF = 0;  # this is the number of columns in the set field table
+	if ( $globalRecord->assignment_type() =~ /gateway/ ) {
+		my $gwhdr = "\n<!-- begin gwoutput table -->\n";
+
+		foreach my $gwfield ( @{ GATEWAY_SET_FIELD_ORDER() } ) {
+
+			# don't show template gateway fields when editing
+			#    set versions
+			next if ( ( $gwfield eq "time_interval" ||
+				    $gwfield eq "versions_per_interval" ) &&
+				  ( $forUsers &&
+				    $userRecord->can('version_id') ) );
+
+			my @fieldData = 
+			    ($self->FieldHTML($userID, $setID, undef, 
+					      $globalRecord, $userRecord, 
+					      $gwfield));
+			if ( @fieldData && defined($fieldData[1]) and 
+			     $fieldData[1] ne '' ) {
+				$nF = @fieldData if ( @fieldData > $nF );
+				$gwFields .= CGI::Tr({}, 
+					CGI::td({}, [@fieldData]));
+		    	}
+		}
+		$gwhdr .= CGI::Tr({},CGI::td({colspan=>$nF}, 
+					     CGI::em("Gateway parameters")))
+		    if ( $nF );
+		$gwFields = "$gwhdr$gwFields\n" .
+			"<!-- end gwoutput table -->\n";
+	}
+
+	# if we have a proctored test, then also generate a proctored 
+	#    set password input 
+	if ( $globalRecord->assignment_type eq 'proctored_gateway' && ! $forUsers ) {
+		my $nfm1 = $nF - 1;
+		$procFields = CGI::Tr({},CGI::td({},''),
+			CGI::td({colspan=>$nfm1},
+				CGI::em("Proctored tests require proctor " .
+					"authorization to start and to " .
+					"grade.  Provide a password to have " .
+					"a single password for all students " .
+					"to start a proctored test.")));
+		# we use a routine other than FieldHTML because of getting
+		#    the default value here
+		my @fieldData = 
+			$self->proctoredFieldHTML($userID, $setID, 
+						  $globalRecord);
+		$procFields .= CGI::Tr({}, 
+			CGI::td({}, [@fieldData]));
+	}
+
+	# finally, figure out what ip selector fields we want to include
+	my @locations = sort {$a cmp $b} ($db->listLocations());
+	$numLocations = @locations;
+
+	# we don't show ip selector fields if we're editing a set version
+	if ( ! defined( $userRecord ) ||
+	     ( defined( $userRecord ) && ! $userRecord->can("version_id") ) ) {
+		if ( ( ! $forUsers && $globalRecord->restrict_ip &&
+		       $globalRecord->restrict_ip ne 'No' ) ||
+		     ( $forUsers && $userRecord->restrict_ip ne 'No' ) ) {
+
+			my @globalLocations = $db->listGlobalSetLocations($setID);
+			# what ip locations should be selected?
+			my @defaultLocations = ();
+			if ( $forUsers && 
+			     ! $db->countUserSetLocations($userID, $setID) ) { 
+				@defaultLocations = @globalLocations;
+				$ipOverride = 0;
+			} elsif ( $forUsers ) {
+				@defaultLocations = $db->listUserSetLocations($userID, $setID);
+				$ipOverride = 1;
+			} else {
+				@defaultLocations = @globalLocations;
+			}
+			my $ipDefaults = join(', ', @globalLocations);
+
+			my $ipSelector = CGI::scrolling_list({
+				-name => "set.$setID.selected_ip_locations",
+				-values => [ @locations ], 
+				-default => [ @defaultLocations ], 
+				-size => 5,
+				-multiple => 'true'});
+
+			my $override = ($forUsers) ? 
+				CGI::checkbox({ type => "checkbox",
+					name => "set.$setID.selected_ip_locations.override",
+					label => "",
+					checked => $ipOverride }) : '';
+			$ipFields .= CGI::Tr({-valign=>'top'},
+					     CGI::td({}, [ $override, 
+						   'Restrict Locations', 
+						   $ipSelector, 
+						   $forUsers ? 
+						   " $ipDefaults" : '', ]
+					),
+			);
+		}
+	}
+	return($gwFields, $ipFields, $numLocations, $procFields);
+}
+
+sub proctoredFieldHTML {
+	my ( $self, $userID, $setID, $globalRecord ) = @_;
+
+	my $r = $self->r;
+	my $db = $r->db;
+
+	# note that this routine assumes that the login proctor password
+	#    is something that can only be changed for the global set
+
+	# if the set doesn't require a login proctor, then we can assume
+	#    that one doesn't exist; otherwise, we need to check the 
+	#    database to find if there's an already defined password
+	my $value = '';
+	if ( $globalRecord->restricted_login_proctor eq 'Yes' &&
+	     $db->existsPassword("set_id:$setID") ) {
+		$value = '********';
+	}
+
+	return( ( '',
+		  'Password (Leave blank for regular proctoring)',
+		  CGI::input({ name=>"set.$setID.restricted_login_proctor_password",
+			       value=>$value,
+			       size=>10,
+		       }),
+		  '' ) );
+}
+
 # creates a popup menu of all possible problem numbers (for possible rearranging)
 sub problem_number_popup {
 	my $num = shift;
@@ -345,22 +721,31 @@
 	my @sortme=();
 	my ($j, $val);
 
+	# keys are current problem numbers, values are target problem numbers
 	foreach $j (keys %newProblemNumbers) {
-		# what happens our first time on this page
+		# we don't want to act unless all problems have been assigned a new problem number, so if any have not, return
 		return "" if (not defined $newProblemNumbers{"$j"});
+		# if the problem has been given a new number, we reduce the "score" of the problem by the original number of the problem
+		# when multiple problems are assigned the same number, this results in the last one ending up first -- FIXME?
 		if ($newProblemNumbers{"$j"} != $j) {
+			# force always gets set if reordering is done, so don't expect to be able to delete a problem,
+			# reorder some other problems, and end up with a hole -- FIXME
 			$force = 1;
 			$val = 1000 * $newProblemNumbers{$j} - $j;
 		} else {
 			$val = 1000 * $newProblemNumbers{$j};
 		}
+		# store a mapping between current problem number and score (based on currnet and new problem number)
 		push @sortme, [$j, $val];
+		# replace new problem numbers in hash with the (global) problems themselves
 		$newProblemNumbers{$j} = $db->getGlobalProblem($setID, $j);
 		die "global $j for set $setID not found." unless $newProblemNumbers{$j};
 	}
 
+	# we don't have to do anything if we're not getting rid of holes
 	return "" unless $force;
 
+	# sort the curr. prob. num./score pairs by score
 	@sortme = sort {$a->[1] <=> $b->[1]} @sortme;
 	# now, for global and each user with this set, loop through problem list
 	#   get all of the problem records
@@ -372,52 +757,74 @@
 	# Now, three stages.  First global values
 
 	for ($j = 0; $j < scalar @sortme; $j++) {
-		if($sortme[$j]->[0] == $j + 1) {
+		if($sortme[$j][0] == $j + 1) {
+			# if the jth problem (according to the new ordering) is in the right place (problem IDs are numbered from 1, hence $j+1)
 			# do nothing
 		} elsif (not defined $newProblemNumbers{$j + 1}) {
-			$newProblemNumbers{$sortme[$j]->[0]}->problem_id($j + 1);
-			$db->addGlobalProblem($newProblemNumbers{$sortme[$j]->[0]});
+			# otherwise, if there's a hole for it, add it there
+			$newProblemNumbers{$sortme[$j][0]}->problem_id($j + 1);
+			$db->addGlobalProblem($newProblemNumbers{$sortme[$j][0]});
 		} else {
-			$newProblemNumbers{$sortme[$j]->[0]}->problem_id($j + 1);
-			$db->putGlobalProblem($newProblemNumbers{$sortme[$j]->[0]});
+			# otherwise, overwrite the data for the problem that's already there with the jth problem's data (with a changed problemID)
+			$newProblemNumbers{$sortme[$j][0]}->problem_id($j + 1);
+			$db->putGlobalProblem($newProblemNumbers{$sortme[$j][0]});
 		}
 	}
 
 	my @setUsers = $db->listSetUsers($setID);
 	my (@problist, $user);
-	my $globalUserID = $db->{set}->{params}->{globalUserID} || '';
 
 	foreach $user (@setUsers) {
-		# if this is gdbm, the global user has been taken care of above.
-		# we can't do it again.  This relies on the global user not having
-		# a blank name.
-		next if $globalUserID eq $user;
+		# grab a copy of each UserProblem for this user. @problist can be sparse (if problems were deleted)
 		for $j (keys %newProblemNumbers) {
 			$problist[$j] = $db->getUserProblem($user, $setID, $j);
-			die " problem $j for set $setID and effective user $user not found" 
-				unless $problist[$j];
 		}
-		# ok, now we have all problem data for $user
 		for($j = 0; $j < scalar @sortme; $j++) { 
-			if ($sortme[$j]->[0] == $j + 1) {
+			if ($sortme[$j][0] == $j + 1) {
+				# same as above -- the jth problem is in the right place, so don't worry about it
 				# do nothing
-			} elsif (not defined $newProblemNumbers{$j + 1}) { 
-				$problist[$sortme[$j]->[0]]->problem_id($j + 1); 
-				$db->addUserProblem($problist[$sortme[$j]->[0]]); 
-			} else { 
-				$problist[$sortme[$j]->[0]]->problem_id($j + 1); 
-				$db->putUserProblem($problist[$sortme[$j]->[0]]); 
-			} 
+			} elsif ($problist[$sortme[$j][0]]) {
+				# we've made sure the user's problem actually exists HERE, since we want to be able to fail gracefullly if it doesn't
+				# the problem with the original conditional below is that %newProblemNumbers maps oldids => global problem record
+				# we need to check if the target USER PROBLEM exists, which is what @problist knows
+				#if (not defined $newProblemNumbers{$j + 1}) {
+				if (not defined $problist[$j+1]) {
+					# same as above -- there's a hole for that problem to go into, so add it in its new place
+					$problist[$sortme[$j][0]]->problem_id($j + 1); 
+					$db->addUserProblem($problist[$sortme[$j][0]]); 
+				} else { 
+					# same as above -- there's a problem already there, so overwrite its data with the data from the jth problem
+					$problist[$sortme[$j][0]]->problem_id($j + 1); 
+					$db->putUserProblem($problist[$sortme[$j][0]]); 
+				} 
+			} else {
+				warn "UserProblem missing for user=$user set=$setID problem=$sortme[$j][0]. This may indicate database corruption.\n";
+				# when a problem doesn't exist in the target slot, a new problem gets added there, but the original problem
+				# never gets overwritten (because there wan't a problem it would have to get exchanged with)
+				# i think this can get pretty complex. consider 1=>2, 2=>3, 3=>4, 4=>1 where problem 1 doesn't exist for some user:
+				# @sortme[$j][0] will contain: 4, 1, 2, 3
+				# - problem 1 will get **added** with the data from problem 4 (because problem 1 doesn't exist for this user)
+				# - problem 2 will get overwritten with the data from problem 1
+				# - problem 3 will get overwritten with the data from problem 2
+				# - nothing will happend to problem 4, since problem 1 doesn't exit
+				# so the solution is to delete problem 4 altogether!
+				# here's the fix:
+				
+				# the data from problem $j+1 was/will be moved to another problem slot,
+				# but there's no problem $sortme[$j][0] to replace it. thus, we delete it now.
+				$db->deleteUserProblem($user, $setID, $j+1);
+			}
 		} 
 	}
 
-
+	# any problems with IDs above $maxNum get deleted -- presumably their data has been copied into problems with lower IDs
 	foreach ($j = scalar @sortme; $j < $maxNum; $j++) {
 		if (defined $newProblemNumbers{$j + 1}) {
 			$db->deleteGlobalProblem($setID, $j+1);
 		}
 	}
 
+	# return a string form of the old problem IDs in the new order (not used by caller, incidentally)
 	return join(', ', map {$_->[0]} @sortme);
 }
 
@@ -477,6 +884,15 @@
 	my $authz     = $r->authz;
 	my $user      = $r->param('user');
 	my $setID   = $r->urlpath->arg("setID");
+
+	## we're now allowing setID to come in as setID,v# to edit a set
+	##    version; catch this first
+	my $editingSetVersion = 0;
+	if ( $setID =~ /,v(\d+)$/ ) {
+	    $editingSetVersion = $1;
+	    $setID =~ s/,v(\d+)$//;
+	}
+
 	my $setRecord = $db->getGlobalSet($setID); # checked
 	die "global set $setID  not found." unless $setRecord;
 
@@ -490,6 +906,9 @@
 	return unless ($authz->hasPermissions($user, "access_instructor_tools"));
 	return unless ($authz->hasPermissions($user, "modify_problem_sets"));
 
+	## if we're editing a versioned set, it only makes sense to be
+	##    editing it for one user
+	return if ( $editingSetVersion && ! $forOneUser );
 
 	my %properties = %{ FIELD_PROPERTIES() };
 
@@ -516,15 +935,15 @@
 	my ($open_date, $due_date, $answer_date);
 	my $error = 0;
 	if (defined $r->param('submit_changes')) {
+		my @names = ("open_date", "due_date", "answer_date");
+		
+		my %dates = map { $_ => $r->param("set.$setID.$_") } @names;
+		%dates = map { 
+			my $unlabel = $undoLabels{$_}->{$dates{$_}}; 
+			$_ => defined $unlabel ? $setRecord->$_ : $self->parseDateTime($dates{$_}) 
+		} @names;
 
-		my $od_param = $r->param("set.$setID.open_date");
-		my $dd_param = $r->param("set.$setID.due_date");
-		my $ad_param = $r->param("set.$setID.answer_date");
-		#my $setRecord = $db->getGlobalSet($setID); # already fetched above --sam
-
-		$open_date = $od_param ? $self->parseDateTime($od_param) : $setRecord->open_date;
-		$due_date = $dd_param ? $self->parseDateTime($dd_param) : $setRecord->due_date;
-		$answer_date = $ad_param ? $self->parseDateTime($ad_param) : $setRecord->answer_date;
+		($open_date, $due_date, $answer_date) = map { $dates{$_} } @names;
 
 		if ($answer_date < $due_date || $answer_date < $open_date) {		
 			$self->addbadmessage("Answers cannot be made available until on or after the due date!");
@@ -536,12 +955,28 @@
 			$error = $r->param('submit_changes');
 		}
 		
-		if ($error) {
-			$self->addbadmessage("No changes were saved!");
+		# make sure the dates are not more than 10 years in the future
+		my $curr_time = time;
+		my $seconds_per_year = 31_556_926;
+		my $cutoff = $curr_time + $seconds_per_year*10;
+		if ($open_date > $cutoff) {
+			$self->addbadmessage("Error: open date cannot be more than 10 years from now in set $setID");
+			$error = $r->param('submit_changes');
+		}
+		if ($due_date > $cutoff) {
+			$self->addbadmessage("Error: due date cannot be more than 10 years from now in set $setID");
+			$error = $r->param('submit_changes');
+		}
+		if ($answer_date > $cutoff) {
+			$self->addbadmessage("Error: answer date cannot be more than 10 years from now in set $setID");
+			$error = $r->param('submit_changes');
 		}
-	}
-
 
+	}
+	if ($error) {
+		$self->addbadmessage("No changes were saved!");
+	}
+	
 	if (defined $r->param('submit_changes') && !$error) {
 
 		#my $setRecord = $db->getGlobalSet($setID); # already fetched above --sam
@@ -551,26 +986,136 @@
 		#####################################################################
 
 		if ($forUsers) {
+			# note that we don't deal with the proctor user
+			#    fields here, with the assumption that it can't
+			#    be possible to change them for users.  this is
+			#    not the most robust treatment of the problem
+			#    (FIXME)
+
+			# DBFIXME use a WHERE clause, iterator
 			my @userRecords = $db->getUserSets(map { [$_, $setID] } @editForUser);
+			# if we're editing a set version, we want to edit
+			#    edit that instead of the userset, so get it
+			#    too.
+			my $userSet = $userRecords[0];
+			my $setVersion = 0;
+			if ( $editingSetVersion ) {
+				$setVersion =
+					$db->getSetVersion($editForUser[0],
+							   $setID,
+							   $editingSetVersion);
+				@userRecords = ( $setVersion );
+			}
+
 			foreach my $record (@userRecords) {
 				foreach my $field ( @{ SET_FIELDS() } ) {
 					next unless canChange($forUsers, $field);
-
 					my $override = $r->param("set.$setID.$field.override");
+
 					if (defined $override && $override eq $field) {
 
 						my $param = $r->param("set.$setID.$field");
 						$param = $properties{$field}->{default} || "" unless defined $param && $param ne "";
-						$param = $undoLabels{$field}->{$param} || $param;
+						my $unlabel = $undoLabels{$field}->{$param};
+						$param = $unlabel if defined $unlabel;
+#						$param = $undoLabels{$field}->{$param} || $param;
 						if ($field =~ /_date/) {
-							$param = $self->parseDateTime($param);
+							$param = $self->parseDateTime($param) unless defined $unlabel;
+						}
+						if (defined($properties{$field}->{convertby}) && $properties{$field}->{convertby}) {
+							$param = $param*$properties{$field}->{convertby};
+						}
+						# special case; does field fill in multiple values?
+						if ( $field =~ /:/ ) {
+							my @values = split(/:/, $param);
+							my @fields = split(/:/, $field);
+							for ( my $i=0; $i<@values; $i++ ) { 
+								my $f=$fields[$i]; 
+								$record->$f($values[$i]); 
+							}
+						} else {
+							$record->$field($param);
 						}
-						$record->$field($param);
 					} else {
-						$record->$field(undef);					
+						####################
+						# FIXME: allow one selector to set multiple fields
+						# 
+						if ( $field =~ /:/ ) {
+							foreach my $f ( split(/:/, $field) ) {
+								$record->$f(undef);
+							}
+						} else {
+							$record->$field(undef);
+						}
+					}
+				
+				}
+				####################
+				# FIXME: this is replaced by our allowing multiple fields to be set by one selector
+				# a check for hiding scores: if we have 
+				#    $set->hide_score eq 'N', we also want 
+				#    $set->hide_score_by_problem eq 'N'
+				# if ( $record->hide_score eq 'N' ) {
+				# 	$record->hide_score_by_problem('N');
+				# }
+				####################
+				if ( $editingSetVersion ) {
+					$db->putSetVersion( $record );
+				} else {
+					$db->putUserSet($record);
+				}
+			}
+
+		#######################################################
+		# Save IP restriction Location information
+		#######################################################
+		# FIXME: it would be nice to have this in the field values
+		#    hash, so that we don't have to assume that we can 
+		#    override this information for users
+
+			## should we allow resetting set locations for set versions?  this
+			##    requires either putting in a new set of database routines
+			##    to deal with the versioned setID, or fudging it at this end
+			##    by manually putting in the versioned ID setID,v#.  neither
+			##    of these seems desirable, so for now it's not allowed
+			if ( ! $editingSetVersion ) {
+				if ( $r->param("set.$setID.selected_ip_locations.override") ) {
+					foreach my $record ( @userRecords ) {
+						my $userID = $record->user_id;
+						my @selectedLocations = $r->param("set.$setID.selected_ip_locations");
+						my @userSetLocations = $db->listUserSetLocations($userID,$setID);
+						my @addSetLocations = ();
+						my @delSetLocations = ();
+						foreach my $loc ( @selectedLocations ) {
+							push( @addSetLocations, $loc ) if ( ! grep( /^$loc$/, @userSetLocations ) );
+						}
+						foreach my $loc ( @userSetLocations ) {
+							push( @delSetLocations, $loc ) if ( ! grep( /^$loc$/, @selectedLocations ) );
+						}
+						# then update the user set_locations
+						foreach ( @addSetLocations ) {
+							my $Loc = $db->newUserSetLocation;
+							$Loc->set_id( $setID );
+							$Loc->user_id( $userID );
+							$Loc->location_id($_);
+							$db->addUserSetLocation($Loc);
+						}
+						foreach ( @delSetLocations ) {
+							$db->deleteUserSetLocation($userID,$setID,$_);
+						}
+					}
+				} else {
+					# if override isn't selected, then we want
+					#    to be sure that there are no
+					#    set_locations_user entries setting around
+					foreach my $record ( @userRecords ) {
+						my $userID = $record->user_id;
+						my @userLocations = $db->listUserSetLocations($userID,$setID);
+						foreach ( @userLocations ) {
+							$db->deleteUserSetLocation($userID,$setID,$_);
+						}
 					}
 				}
-				$db->putUserSet($record);
 			}
 		} else {
 			foreach my $field ( @{ SET_FIELDS() } ) {
@@ -578,20 +1123,161 @@
 
 				my $param = $r->param("set.$setID.$field");
 				$param = $properties{$field}->{default} || "" unless defined $param && $param ne "";
-				$param = $undoLabels{$field}->{$param} || $param;
+
+				my $unlabel = $undoLabels{$field}->{$param};
+				$param = $unlabel if defined $unlabel;
 				if ($field =~ /_date/) {
-					$param = $self->parseDateTime($param);
+					$param = $self->parseDateTime($param) unless defined $unlabel;
+				}
+				if (defined($properties{$field}->{convertby}) && $properties{$field}->{convertby} && $param) {
+					$param = $param*$properties{$field}->{convertby};
+				}
+				# special case; does field fill in multiple values?
+				if ( $field =~ /:/ ) {
+					my @values = split(/:/, $param);
+					my @fields = split(/:/, $field);
+					for ( my $i=0; $i<@fields; $i++ ) { 
+						my $f = $fields[$i];
+						$setRecord->$f($values[$i]); 
+					}
+				} else {
+					$setRecord->$field($param);
 				}
-				$setRecord->$field($param);
 			}
+####################
+# FIXME: this is replaced by our setting both hide_score and hide_score_by_problem
+#    with a single drop down
+# 
+# 			# a check for hiding scores: if we have 
+# 			#    $set->hide_score eq 'N', we also want 
+# 			#    $set->hide_score_by_problem eq 'N', and if it's
+# 			#    changed to 'Y' and hide_score_by_problem is Null,
+# 			#    give it a value 'N'
+# 			if ( $setRecord->hide_score eq 'N' ||
+# 			     ( ! defined($setRecord->hide_score_by_problem) ||
+# 			       $setRecord->hide_score_by_problem eq '' ) ) {
+# 				$setRecord->hide_score_by_problem('N');
+# 			}
+####################
 			$db->putGlobalSet($setRecord);
-		}
 
+		#######################################################
+		# Save IP restriction Location information
+		#######################################################
+
+			if ( defined($r->param("set.$setID.restrict_ip")) and $r->param("set.$setID.restrict_ip") ne 'No' ) {
+				my @selectedLocations = $r->param("set.$setID.selected_ip_locations");
+				my @globalSetLocations = $db->listGlobalSetLocations($setID);
+				my @addSetLocations = ();
+				my @delSetLocations = ();
+				foreach my $loc ( @selectedLocations ) {
+					push( @addSetLocations, $loc ) if ( ! grep( /^$loc$/, @globalSetLocations ) );
+				}
+				foreach my $loc ( @globalSetLocations ) {
+					push( @delSetLocations, $loc ) if ( ! grep( /^$loc$/, @selectedLocations ) );
+				}
+				# then update the global set_locations
+				foreach ( @addSetLocations ) {
+					my $Loc = $db->newGlobalSetLocation;
+					$Loc->set_id( $setID );
+					$Loc->location_id($_);
+					$db->addGlobalSetLocation($Loc);
+				}
+				foreach ( @delSetLocations ) {
+					$db->deleteGlobalSetLocation($setID,$_);
+				}
+			} else {
+				my @globalSetLocations = $db->listGlobalSetLocations($setID);
+				foreach ( @globalSetLocations ) {
+					$db->deleteGlobalSetLocation($setID,$_);
+				}
+			}
+
+		#######################################################
+		# Save proctored problem proctor user information
+		#######################################################
+			if ($r->param("set.$setID.restricted_login_proctor_password") &&
+			    $setRecord->assignment_type eq 'proctored_gateway') {
+				# in this case we're adding a set-level proctor
+				#    or updating the password
+
+				my $procID = "set_id:$setID";
+				my $pass = $r->param("set.$setID.restricted_login_proctor_password");
+				# should we carefully check in this case that
+				#    the user and password exist?  the code 
+				#    in the add stanza is pretty careful to 
+				#    be sure that there's a one-to-one 
+				#    correspondence between the existence of 
+				#    the user and the setting of the set
+				#    restricted_login_proctor field, so we 
+				#    assume that just checking the latter 
+				#    here is sufficient.
+				if ( $setRecord->restricted_login_proctor eq 'Yes' ) {
+					# in this case we already have a set
+					#    level proctor, and so should be 
+					#    resetting the password
+					if ( $pass ne '********' ) {
+						# then we submitted a new 
+						#    password, so save it
+						my $dbPass;
+						eval { $dbPass = $db->getPassword($procID) };
+						if ( $@ ) {
+							$self->addbadmessage("Error getting old set-proctor password from the database: $@.  No update to the password was done.");
+						} else {
+							$dbPass->password(cryptPassword($pass));
+							$db->putPassword($dbPass);
+						}
+					}
 
+				} else {
+					$setRecord->restricted_login_proctor('Yes');
+					my $procUser = $db->newUser();
+					$procUser->user_id($procID);
+					$procUser->last_name("Proctor");
+					$procUser->first_name("Login");
+					$procUser->student_id("loginproctor");
+					$procUser->status($ce->status_name_to_abbrevs('Proctor'));
+					my $procPerm = $db->newPermissionLevel;
+					$procPerm->user_id($procID);
+					$procPerm->permission($ce->{userRoles}->{login_proctor});
+					my $procPass = $db->newPassword;
+					$procPass->user_id($procID);
+					$procPass->password(cryptPassword($pass));
+					# put these into the database
+					eval { $db->addUser($procUser) };
+					if ( $@ ) { 
+						$self->addbadmessage("Error " .
+							"adding set-level " .
+							"proctor: $@");
+					} else {
+						$db->addPermissionLevel($procPerm);
+						$db->addPassword($procPass);
+					}
+
+					# and set the restricted_login_proctor
+					#    set field
+					$db->putGlobalSet( $setRecord );
+				}
+
+			} else {
+				# if the parameter isn't set, or if the assignment
+				#    type is not 'proctored_gateway', then we need to be 
+				#    sure that there's no set-level proctor defined
+				if ( $setRecord->restricted_login_proctor eq 'Yes' ) {
+
+					$setRecord->restricted_login_proctor('No');
+					$db->deleteUser( "set_id:$setID" );
+					$db->putGlobalSet( $setRecord );
+
+				}
+			}
+		}
+		
 		#####################################################################
 		# Save problem information
 		#####################################################################
 
+		# DBFIXME use a WHERE clause, iterator?
 		my @problemIDs = sort { $a <=> $b } $db->listGlobalProblems($setID);;
 		my @problemRecords = $db->getGlobalProblems(map { [$setID, $_] } @problemIDs);
 		foreach my $problemRecord (@problemRecords) {
@@ -604,8 +1290,18 @@
 				# in the GlobalProblem record or for fields unique to the UserProblem record.
 								
 				my @userIDs = @editForUser;
-				my @userProblemIDs = map { [$_, $setID, $problemID] } @userIDs;
-				my @userProblemRecords = $db->getUserProblems(@userProblemIDs);
+
+				my @userProblemRecords;
+				if ( ! $editingSetVersion ) {
+					my @userProblemIDs = map { [$_, $setID, $problemID] } @userIDs;
+					# DBFIXME where clause? iterator?
+					@userProblemRecords = $db->getUserProblems(@userProblemIDs);
+				} else {
+					## (we know that we're only editing for one user)
+					@userProblemRecords =
+						( $db->getMergedProblemVersion( $userIDs[0], $setID, $editingSetVersion, $problemID ) );
+				}
+
 				foreach my $record (@userProblemRecords) {
 
 					my $changed = 0; # keep track of any changes, if none are made, avoid unnecessary db accesses
@@ -617,7 +1313,8 @@
 
 							my $param = $r->param("problem.$problemID.$field");
 							$param = $properties{$field}->{default} || "" unless defined $param && $param ne "";
-							$param = $undoLabels{$field}->{$param} || $param;
+							my $unlabel = $undoLabels{$field}->{$param};
+							$param = $unlabel if defined $unlabel;
 							$changed ||= changed($record->$field, $param);
 							$record->$field($param);
 						} else {
@@ -632,11 +1329,16 @@
 
 						my $param = $r->param("problem.$problemID.$field");
 						$param = $properties{$field}->{default} || "" unless defined $param && $param ne "";
-						$param = $undoLabels{$field}->{$param} || $param;
+						my $unlabel = $undoLabels{$field}->{$param};
+						$param = $unlabel if defined $unlabel;
 						$changed ||= changed($record->$field, $param);
 						$record->$field($param);
 					}
-					$db->putUserProblem($record) if $changed;
+					if ( ! $editingSetVersion ) {
+						$db->putUserProblem($record) if $changed;
+					} else {
+						$db->putProblemVersion($record) if $changed;
+					}
 				}
 			} else { 
 				# Since we're editing for ALL set users, we will make changes to the GlobalProblem record.
@@ -650,7 +1352,8 @@
 
 					my $param = $r->param("problem.$problemID.$field");
 					$param = $properties{$field}->{default} || "" unless defined $param && $param ne "";
-					$param = $undoLabels{$field}->{$param} || $param;
+					my $unlabel = $undoLabels{$field}->{$param};
+					$param = $unlabel if defined $unlabel;
 					$changed ||= changed($problemRecord->$field, $param);
 					$problemRecord->$field($param);
 				}
@@ -672,6 +1375,7 @@
 				}
 
 				if (keys %useful) {
+					# DBFIXME where clause, iterator
 					my @userIDs = $db->listProblemUsers($setID, $problemID);
 					my @userProblemIDs = map { [$_, $setID, $problemID] } @userIDs;
 					my @userProblemRecords = $db->getUserProblems(@userProblemIDs);
@@ -682,7 +1386,8 @@
 
 							my $param = $r->param("problem.$problemID.$field");
 							$param = $properties{$field}->{default} || "" unless defined $param && $param ne "";
-							$param = $undoLabels{$field}->{$param} || $param;
+							my $unlabel = $undoLabels{$field}->{$param};
+							$param = $unlabel if defined $unlabel;
 							$changed ||= changed($record->$field, $param);
 							$record->$field($param);
 						}
@@ -692,29 +1397,94 @@
 			}
 		}
 		
-		# Delete all problems marked for deletion
+		# Mark the specified problems as correct for all users (not applicable when editing a set
+		#    version, because this only shows up when editing for users or editing the
+		#    global set/problem, not for one user)
+		foreach my $problemID ($r->param('markCorrect')) {
+			# DBFIXME where clause, iterator
+			my @userProblemIDs = map { [$_, $setID, $problemID] } ($forUsers ? @editForUser : $db->listProblemUsers($setID, $problemID));
+			# if the set is not a gateway set, this requires going through the
+			#    user_problems and resetting their status; if it's a gateway set,
+			#    then we have to go through every *version* of every user_problem.
+			#    it may be that there is an argument for being able to get() all
+			#    problem versions for all users in one database call.  The current
+			#    code may be slow for large classes.
+			if ( $setRecord->assignment_type !~ /gateway/ ) {
+				my @userProblemRecords = $db->getUserProblems(@userProblemIDs);
+				foreach my $record (@userProblemRecords) {
+					if (defined $record && ($record->status eq "" || $record->status < 1)) {
+						$record->status(1);
+						$record->attempted(1);
+						$db->putUserProblem($record);
+					}
+				}
+			} else {
+				my @userIDs = ( $forUsers ) ? @editForUser : $db->listProblemUsers($setID, $problemID);
+				foreach my $uid ( @userIDs ) {
+					my @versions = $db->listSetVersions( $uid, $setID );
+					my @userProblemVersionIDs = 
+						map{ [ $uid, $setID, $_, $problemID ]} @versions;
+					my @userProblemVersionRecords = $db->getProblemVersions(@userProblemVersionIDs);
+					foreach my $record (@userProblemVersionRecords) { 
+						if (defined $record && ($record->status eq "" || $record->status < 1)) {
+							$record->status(1);
+							$record->attempted(1);
+							$db->putProblemVersion($record);
+						}
+					}
+				}
+			}
+		}
+		
+		# Delete all problems marked for deletion (not applicable when editing
+		#    for users)
 		foreach my $problemID ($r->param('deleteProblem')) {
 			$db->deleteGlobalProblem($setID, $problemID);
 		}
 		
+		#####################################################################
+		# Add blank problem if needed
+		#####################################################################
+		if (defined($r->param("add_blank_problem") ) and $r->param("add_blank_problem") == 1) {
+		   # get number of problems to add and clean the entry
+		    my $newBlankProblems = (defined($r->param("add_n_problems")) ) ? $r->param("add_n_problems") :1;
+		    $newBlankProblems = int($newBlankProblems);
+		    my $MAX_NEW_PROBLEMS = 20;
+		    if ($newBlankProblems >=1 and $newBlankProblems <= $MAX_NEW_PROBLEMS ) {
+				foreach my $newProb (1..$newBlankProblems) {
+						my $targetProblemNumber   =  1+ WeBWorK::Utils::max( $self->r->db->listGlobalProblems($setID));
+						##################################################
+						# make local copy of the blankProblem
+						##################################################
+						my $blank_file_path       =  $ce->{webworkFiles}->{screenSnippets}->{blankProblem};
+						my $problemContents       =  WeBWorK::Utils::readFile($blank_file_path);
+						my $new_file_path         =  "set$setID/".BLANKPROBLEM();
+						my $fullPath              =  WeBWorK::Utils::surePathToFile($ce->{courseDirs}->{templates},'/'.$new_file_path);
+						local(*TEMPFILE);
+						open(TEMPFILE, ">$fullPath") or warn "Can't write to file $fullPath";
+						print TEMPFILE $problemContents;
+						close(TEMPFILE);
+						
+						#################################################
+						# Update problem record
+						#################################################
+						my $problemRecord  = $self->addProblemToSet(
+								   setName        => $setID,
+								   sourceFile     => $new_file_path, 
+								   problemID      => $targetProblemNumber, #added to end of set
+						);
+						$self->assignProblemToAllSetUsers($problemRecord);
+						$self->addgoodmessage("Added $new_file_path to ". $setID. " as problem $targetProblemNumber") ;
+				}
+			} else {
+				$self->addbadmessage("Could not add $newBlankProblems problems to this set.  The number must be between 1 and $MAX_NEW_PROBLEMS");
+			}
+		}
+		
 		# Sets the specified header to "" so that the default file will get used.
 		foreach my $header ($r->param('defaultHeader')) {
 			$setRecord->$header("");
 		}
-		
-		# Mark the specified problems as correct for all users
-		foreach my $problemID ($r->param('markCorrect')) {
-			my @userProblemIDs = map { [$_, $setID, $problemID] } ($forUsers ? @editForUser : $db->listProblemUsers($setID, $problemID));
-			my @userProblemRecords = $db->getUserProblems(@userProblemIDs);
-			foreach my $record (@userProblemRecords) {
-$self->addbadmessage($record->user_id);
-				if (defined $record && ($record->status eq "" || $record->status < 1)) {
-					$record->status(1);
-					$record->attempted(1);
-					$db->putUserProblem($record);
-				}
-			}
-		}
 	}	
 	
 # Leftover code from when there were up/down buttons
@@ -749,10 +1519,9 @@
 		# if the current naming scheme is changed/broken, this could reek havoc
 		# on all kinds of things
 		foreach my $param ($r->param) {
-			$r->param($param, "") if $param =~ /^(set|problem|header)\./;
+			$r->param($param, "") if $param =~ /^(set|problem|header)\./  && $param !~ /displaymode/;
 		}
 	}
-	
 }
 
 # helper method for debugging
@@ -788,7 +1557,6 @@
 	my $forOneUser = $forUsers == 1;
 	
 	my $howManyCan = $properties{$field}->{override};
-	
 	return 0 if $howManyCan eq "none";
 	return 1 if $howManyCan eq "any";	
 	return 1 if $howManyCan eq "one" && $forOneUser;
@@ -804,7 +1572,9 @@
 	my $ce = $r->ce;
 
 	return "No source file specified" unless $file;
-	$file = $ce->{courseDirs}->{templates} . '/' . $file unless $file =~ m|^/|;
+	return "Problem source is drawn from a grouping set" if $file =~ /^group/;
+#	$file = $ce->{courseDirs}->{templates} . '/' . $file unless $file =~ m|^/|; # bug: 1725 allows access to all files e.g. /etc/passwd
+	$file = $ce->{courseDirs}->{templates} . '/' . $file ; # only files in template directory can be accessed
 
 	my $text = "This source file ";
 	my $fileError;
@@ -815,6 +1585,11 @@
 	return $text . "is not a plain file!";
 }
 
+# don't show view options -- we provide display mode controls for headers/problems separately
+sub options {
+	return "";
+}
+
 # Creates two separate tables, first of the headers, and the of the problems in a given set
 # If one or more users are specified in the "editForUser" param, only the data for those users
 # becomes editable, not all the data
@@ -829,6 +1604,16 @@
 	my $urlpath     = $r->urlpath;
 	my $courseID    = $urlpath->arg("courseID");
 	my $setID       = $urlpath->arg("setID");
+
+	## we're now allowing setID to come in as setID,v# to edit a set
+	##    version; catch this first
+	my $editingSetVersion = 0;
+	my $fullSetID = $setID;
+	if ( $setID =~ /,v(\d+)$/ ) {
+	    $editingSetVersion = $1;
+	    $setID =~ s/,v(\d+)$//;
+	}
+
 	my $setRecord   = $db->getGlobalSet($setID) or die "No record for global set $setID.";
 
 	my $userRecord = $db->getUser($userID) or die "No record for user $userID.";
@@ -841,18 +1626,22 @@
 
 	my @editForUser = $r->param('editForUser');
 
+	return CGI::div({class=>"ResultsWithError"}, "Versions of a set can only be " .
+			"edited for one user at a time.") if ( $editingSetVersion && @editForUser != 1 );
+
 	# Check that every user that we're editing for has a valid UserSet
 	my @assignedUsers;
 	my @unassignedUsers;
 	if (scalar @editForUser) {
 		foreach my $ID (@editForUser) {
+			# DBFIXME iterator
 			if ($db->getUserSet($ID, $setID)) {
 				unshift @assignedUsers, $ID;
 			} else {
 				unshift @unassignedUsers, $ID;
 			}
 		}
-		@editForUser = @assignedUsers;
+		@editForUser = sort @assignedUsers;
 		$r->param("editForUser", \@editForUser);
 		
 		if (scalar @editForUser && scalar @unassignedUsers) {
@@ -862,20 +1651,31 @@
 			print CGI::div({class=>"ResultsWithError"}, "Global set data will be shown instead of user specific data");
 		}		
 	}
-	
+
 	# some useful booleans
 	my $forUsers    = scalar(@editForUser);
 	my $forOneUser  = $forUsers == 1;
 
+	# and check that if we're editing a set version for a user, that
+	#    it exists as well
+	if ( $editingSetVersion && ! $db->existsSetVersion( $editForUser[0], $setID, $editingSetVersion ) ) {
+		return CGI::div({class=>"ResultsWithError"}, "The set-version ($setID, version $editingSetVersion) is not assigned to user $editForUser[0].");
+	}
+
 	# If you're editing for users, initially their records will be different but
 	# if you make any changes to them they will be the same.
 	# if you're editing for one user, the problems shown should be his/hers
 	my $userToShow        = $forUsers ? $editForUser[0] : $userID;
+
+	# a useful gateway variable
+	my $isGatewaySet = ( $setRecord->assignment_type =~ /gateway/ ) ? 1 : 0;
 	
+	# DBFIXME no need to get ID lists -- counts would be fine
 	my $userCount        = $db->listUsers();
-	my $setCount         = $db->listGlobalSets() if $forOneUser;
+	my $setCount         = $db->listGlobalSets(); # if $forOneUser;
 	my $setUserCount     = $db->countSetUsers($setID);
-	my $userSetCount     = $db->countUserSets($editForUser[0]) if $forOneUser;
+# if $forOneUser;
+	my $userSetCount     = ($forOneUser && @editForUser) ? $db->countUserSets($editForUser[0]) : 0;
 
 	
 	my $editUsersAssignedToSetURL = $self->systemLink(
@@ -884,13 +1684,13 @@
                   courseID => $courseID, setID => $setID));
 	my $editSetsAssignedToUserURL = $self->systemLink(
 	      $urlpath->newFromModule(
-                "WeBWorK::ContentGenerator::Instructor::SetsAssignedToUser",
+                "WeBWorK::ContentGenerator::Instructor::UserDetail",
                   courseID => $courseID, userID => $editForUser[0])) if $forOneUser;
 
 
 	my $setDetailPage  = $urlpath -> newFromModule($urlpath->module, courseID => $courseID, setID => $setID);
-	my $setDetailURL   = $self->systemLink($setDetailPage,authen=>0);
-
+	my $fullsetDetailPage  = $urlpath -> newFromModule($urlpath->module, courseID => $courseID, setID => $fullSetID);
+	my $setDetailURL   = $self->systemLink($fullsetDetailPage, authen=>0);
 
 	my $userCountMessage = CGI::a({href=>$editUsersAssignedToSetURL}, $self->userCountMessage($setUserCount, $userCount));
 	my $setCountMessage = CGI::a({href=>$editSetsAssignedToUserURL}, $self->setCountMessage($userSetCount, $setCount)) if $forOneUser;
@@ -899,12 +1699,62 @@
 	$setCountMessage  = "The user $editForUser[0] has been assigned " . $setCountMessage . "." if $forOneUser;
 
 	if ($forUsers) {
-		print CGI::p("$userCountMessage  Editing user-specific overrides for ". CGI::b(join ", ", @editForUser));
-		if ($forOneUser) {
-			print CGI::p($setCountMessage);
+	    ##############################################
+		# calculate links for the users being edited:
+		##############################################
+		my @userLinks = ();
+		foreach my $userID (@editForUser) {
+			my $u = $db->getUser($userID);
+			my $email_address = $u->email_address;
+			my $line = $u->last_name.", " . $u->first_name . "&nbsp;&nbsp;(" .
+				CGI::a({-href=>"mailto:$email_address"},"email "). $u->user_id .
+				"). ";
+			if ( ! $editingSetVersion ) {
+				$line .= "Assigned to ";
+				my $editSetsAssignedToUserURL = $self->systemLink(
+					$urlpath->newFromModule(
+						"WeBWorK::ContentGenerator::Instructor::UserDetail",
+                  				courseID => $courseID, userID => $u->user_id));
+            			$line .= CGI::a({href=>$editSetsAssignedToUserURL}, 
+                     			$self->setCountMessage($db->countUserSets($u->user_id), 
+						$setCount));
+			} else {
+				my $editSetLink = $self->systemLink( $setDetailPage,
+					params=>{effectiveUser=>$u->user_id,
+						 editForUser  =>$u->user_id} );
+				$line .= "Edit set " . CGI::a({href=>$editSetLink},$setID) .
+					" for this user.";
+			}
+			unshift @userLinks,$line;
 		}
+		@userLinks = sort @userLinks;
+	
+		# handy messages when editing gateway sets
+		my $gwmsg = ( $isGatewaySet && ! $editingSetVersion ) ?
+			CGI::br() . CGI::em("To edit a specific student version of this set, " .
+			    "edit (all of) her/his assigned sets.") : "";
+		my $vermsg = ( $editingSetVersion ) ? ", test $editingSetVersion" : "";
+
+		print CGI::table({border=>2,cellpadding=>10}, 
+		    CGI::Tr({},
+				CGI::td([
+					 "Editing problem set ".CGI::strong($setID . $vermsg)." data for these individual students:".CGI::br(). 
+					                CGI::strong(join CGI::br(), @userLinks),
+					CGI::a({href=>$self->systemLink($setDetailPage) },"Edit set ".CGI::strong($setID)." data for ALL students assigned to this set.") . $gwmsg,
+				
+				])
+			)
+		);
 	} else {
-		print CGI::p($userCountMessage);
+		print CGI::table({border=>2,cellpadding=>10}, 
+		    CGI::Tr({},
+				CGI::td([
+					"This set ".CGI::strong($setID)." is assigned to ".$self->userCountMessage($setUserCount, $userCount).'.' ,
+					'Edit '.CGI::a({href=>$editUsersAssignedToSetURL},'individual versions '). "of set $setID.",
+				
+				])
+			)
+		);
 	}
 	
 	# handle renumbering of problems if necessary
@@ -933,8 +1783,7 @@
 	#####################################################################
 	
 	my $templates = $r->ce->{courseDirs}->{templates};
-	my %probLibs = %{ $r->ce->{courseFiles}->{problibs} };
-	my $skip = join("|", keys %probLibs);
+	my $skip = join("|", keys %{ $r->ce->{courseFiles}->{problibs} });
 
 	my @headerFileList = listFilesRecursive(
 		$templates,
@@ -944,14 +1793,6 @@
 		1, 				# prune against path relative to $templates
 	);
 
-	# this just takes too much time to search
-#	my @problemFileList = listFilesRecursive(
-#		$templates,
-#		qr/\.pg$/i,			# problem files don't say problem
-#		qr/^(?:$skip|CVS)$/, 	# prune these directories
-#		0, 				# match against file name only
-#		1, 				# prune against path relative to $templates
-#	);
 
 	# Display a useful warning message
 	if ($forUsers) {
@@ -983,6 +1824,13 @@
 	# pass it to FieldTable, so FieldTable can pass it to FieldHTML, so
 	# FieldHTML doesn't have to fetch it itself.
 	my $userSetRecord = $db->getUserSet($userToShow, $setID);
+
+	my $templateUserSetRecord;
+	# send in the set version if we're editing for versions
+	if ( $editingSetVersion ) {
+		$templateUserSetRecord = $userSetRecord;
+		$userSetRecord = $db->getSetVersion( $userToShow, $setID, $editingSetVersion );
+	}
 	
 	print CGI::Tr({}, CGI::td({}, [
 		$self->FieldTable($userToShow, $setID, undef, $setRecord, $userSetRecord),
@@ -1014,17 +1862,31 @@
 		my %header_html;
 		
 		my %error;
+		my $this_set = $db->getMergedSet($userToShow, $setID);
+		my $guaranteed_set = $this_set;
+		if ( ! $guaranteed_set ) { 
+			# in the header loop we need to have a set that 
+			#    we know exists, so if the getMergedSet failed
+			#    (that is, the set isn't assigned to the 
+			#    the current user), we get the global set instead
+			# $guaranteed_set = $db->getGlobalSet( $setID );
+			$guaranteed_set = $setRecord;
+		}
+
 		foreach my $header (@headers) {
+
 			my $headerFile = $r->param("set.$setID.$header") || $setRecord->{$header} || $headerDefaults{$header};
 
 			$error{$header} = $self->checkFile($headerFile);
+
 			unless ($error{$header}) {
-				my @temp = renderProblems(	r=> $r, 
-								user => $db->getUser($userToShow),
-								displayMode=> $default_header_mode,
-								problem_number=> 0,
-								this_set => $db->getMergedSet($userToShow, $setID),
-								problem_list => [$headerFile],
+				my @temp = renderProblems(
+					r=> $r, 
+					user => $db->getUser($userToShow),
+					displayMode=> $default_header_mode,
+					problem_number=> 0,
+					this_set => $this_set,
+					problem_list => [$headerFile],
 				);
 				$header_html{$header} = $temp[0];
 			}
@@ -1038,11 +1900,25 @@
 			my $viewHeaderPage = $urlpath->new(type => $headerModules{$header}, args => { courseID => $courseID, setID => $setID });
 			my $viewHeaderLink = $self->systemLink($viewHeaderPage);
 			
+			# this is a bit of a hack; the set header isn't shown
+			#    for gateway tests, and we run into trouble trying to 
+			#    edit/view it in this context, so we don't show this 
+			#    field for gateway tests
+			if ( $header eq 'set_header' && 
+		     	     $guaranteed_set->assignment_type =~ /gateway/ ) {
+				print CGI::Tr({}, CGI::td({}, 
+					      [ "Set Header", 
+					     	"Set headers are not used in " .
+						"display of gateway tests."]));
+				next;
+			}
+
+
 			print CGI::Tr({}, CGI::td({}, [
 				CGI::start_table({border => 0, cellpadding => 0}) . 
 					CGI::Tr({}, CGI::td({}, $properties{$header}->{name})) . 
-					CGI::Tr({}, CGI::td({}, CGI::a({href => $editHeaderLink}, "Edit it"))) .
-					CGI::Tr({}, CGI::td({}, CGI::a({href => $viewHeaderLink}, "View it"))) .
+					CGI::Tr({}, CGI::td({}, CGI::a({href => $editHeaderLink, target=>"WW_Editor"}, "Edit it"))) .
+					CGI::Tr({}, CGI::td({}, CGI::a({href => $viewHeaderLink, target=>"WW_View"}, "View it"))) .
 #					CGI::Tr({}, CGI::td({}, CGI::checkbox({name => "defaultHeader", value => $header, label => "Use Default"}))) .
 				CGI::end_table(),
 #				"",
@@ -1080,17 +1956,26 @@
 
 	my @problemIDList = sort { $a <=> $b } $db->listGlobalProblems($setID);
 	
+	# DBFIXME use iterators instead of getting all at once
+	
 	# get global problem records for all problems in one go
 	my %GlobalProblems;
 	my @globalKeypartsRef = map { [$setID, $_] } @problemIDList;
+	# DBFIXME shouldn't need to get key list here
 	@GlobalProblems{@problemIDList} = $db->getGlobalProblems(@globalKeypartsRef);
 	
 	# if needed, get user problem records for all problems in one go
 	my (%UserProblems, %MergedProblems);
 	if ($forOneUser) {
 		my @userKeypartsRef = map { [$editForUser[0], $setID, $_] } @problemIDList;
+		# DBFIXME shouldn't need to get key list here
 		@UserProblems{@problemIDList} = $db->getUserProblems(@userKeypartsRef);
-		@MergedProblems{@problemIDList} = $db->getMergedProblems(@userKeypartsRef);
+		if ( ! $editingSetVersion ) {
+			@MergedProblems{@problemIDList} = $db->getMergedProblems(@userKeypartsRef);
+		} else {
+			my @userversionKeypartsRef = map { [$editForUser[0], $setID, $editingSetVersion, $_] } @problemIDList;
+			@MergedProblems{@problemIDList} = $db->getMergedProblemVersions(@userversionKeypartsRef);
+		}
 	}
 	
 	if (scalar @problemIDList) {
@@ -1106,6 +1991,7 @@
 		
 		my %shownYet;
 		my $repeatFile;
+
 		foreach my $problemID (@problemIDList) {
 		
 			my $problemRecord;
@@ -1116,21 +2002,47 @@
 				#$problemRecord = $db->getGlobalProblem($setID, $problemID);
 				$problemRecord = $GlobalProblems{$problemID}; # already fetched above --sam
 			}
-			
+
 			#$self->addgoodmessage("");
 			#$self->addbadmessage($problemRecord->toString());
-			
-			
-			my $editProblemPage = $urlpath->new(type => 'instructor_problem_editor_withset_withproblem', args => { courseID => $courseID, setID => $setID, problemID => $problemID });
-			my $editProblemLink = $self->systemLink($editProblemPage, params => { make_local_copy => 0 });
 
+			# when we're editing a set version, we want to be sure to
+			#    use the merged problem in the edit, because we could
+			#    be using problem groups (for which the problem is generated
+			#    and then stored in the problem version)
+			my $problemToShow = ( $editingSetVersion ) ?
+				$MergedProblems{$problemID} : $UserProblems{$problemID};
+
+			my ( $editProblemPage, $editProblemLink, $viewProblemPage,
+			     $viewProblemLink );
+			if ( $isGatewaySet ) {
+				$editProblemPage = $urlpath->new(type =>'instructor_problem_editor_withset_withproblem', args => { courseID => $courseID, setID => $fullSetID, problemID => $problemID });
+				$editProblemLink = $self->systemLink($editProblemPage, params => { make_local_copy => 0 });
+				$viewProblemPage =
+					$urlpath->new(type =>'gateway_quiz',
+						      args => { courseID => $courseID,
+								setID => "Undefined_Set",
+								problemID => "1" } );
+
+				my $seed = $problemToShow ? $problemToShow->problem_seed : "";
+				my $file = $problemToShow ? $problemToShow->source_file : 
+					$GlobalProblems{$problemID}->source_file;
+
+				$viewProblemLink =
+					$self->systemLink( $viewProblemPage,
+						params => { effectiveUser =>
+							    ($forOneUser ? $editForUser[0] : $userID),
+							    problemSeed => $seed,
+							    sourceFilePath => $file });
+			} else {
+				$editProblemPage = $urlpath->new(type => 'instructor_problem_editor_withset_withproblem', args => { courseID => $courseID, setID => $fullSetID, problemID => $problemID });
+				$editProblemLink = $self->systemLink($editProblemPage, params => { make_local_copy => 0 });
 			# FIXME: should we have an "act as" type link here when editing for multiple users?		
-			my $viewProblemPage = $urlpath->new(type => 'problem_detail', args => { courseID => $courseID, setID => $setID, problemID => $problemID });
-			my $viewProblemLink = $self->systemLink($viewProblemPage, params => { effectiveUser => ($forOneUser ? $editForUser[0] : $userID)});
-
-			my @fields = @{ PROBLEM_FIELDS() };
-			push @fields, @{ USER_PROBLEM_FIELDS() } if $forOneUser;
+				$viewProblemPage = $urlpath->new(type => 'problem_detail', args => { courseID => $courseID, setID => $setID, problemID => $problemID });
+				$viewProblemLink = $self->systemLink($viewProblemPage, params => { effectiveUser => ($forOneUser ? $editForUser[0] : $userID)});
+			}
 
+	
 			my $problemFile = $r->param("problem.$problemID.source_file") || $problemRecord->source_file;
 
 			# warn of repeat problems
@@ -1138,31 +2050,45 @@
 				$repeatFile = "This problem uses the same source file as number " . $shownYet{$problemFile} . ".";
 			} else {
 				$shownYet{$problemFile} = $problemID;
+				$repeatFile = "";
 			}
 			
 			my $error = $self->checkFile($problemFile);
+			my $this_set = $db->getMergedSet($userToShow, $setID);
 			my @problem_html;
 			unless ($error) {
-				@problem_html = renderProblems(	r=> $r, 
-								user => $db->getUser($userToShow),
-								displayMode=> $default_problem_mode,
-								problem_number=> $problemID,
-								this_set => $db->getMergedSet($userToShow, $setID),
-								problem_seed => $forOneUser ? $problemRecord->problem_seed : 0,
-								problem_list => [$problemRecord->source_file],
+				@problem_html = renderProblems(
+					r=> $r, 
+					user => $db->getUser($userToShow),
+					displayMode=> $default_problem_mode,
+					problem_number=> $problemID,
+					this_set => $this_set,
+					problem_seed => $forOneUser ? $problemRecord->problem_seed : 0,
+					problem_list => [$problemRecord->source_file],
 				);
 			}
 
+			# we want to show the "Try It" and "Edit It" links if there's a 
+			#    well defined problem to view; this is when we're editing a 
+			#    homework set, or if we're editing a gateway set version, or 
+			#    if we're editing a gateway set and the problem is not a 
+			#    group problem
+			my $showLinks = ( ! $isGatewaySet || 
+					  ( $editingSetVersion || $problemFile !~ /^group/ ));
+
+
 			print CGI::Tr({}, CGI::td({}, [
 				CGI::start_table({border => 0, cellpadding => 1}) .
 					CGI::Tr({}, CGI::td({}, problem_number_popup($problemID, $maxProblemNumber))) .
-					CGI::Tr({}, CGI::td({}, CGI::a({href => $editProblemLink}, "Edit it"))) .
-					CGI::Tr({}, CGI::td({}, CGI::a({href => $viewProblemLink}, "Try it" . ($forOneUser ? " (as $editForUser[0])" : "")))) .
+					CGI::Tr({}, CGI::td({}, 
+							    $showLinks ? CGI::a({href => $editProblemLink, target=>"WW_Editor"}, "Edit it") : "" )) .
+					CGI::Tr({}, CGI::td({}, 
+							    $showLinks ? CGI::a({href => $viewProblemLink, target=>"WW_View"}, "Try it" . ($forOneUser ? " (as $editForUser[0])" : "")) : "" )) .
 					($forUsers ? "" : CGI::Tr({}, CGI::td({}, CGI::checkbox({name => "deleteProblem", value => $problemID, label => "Delete it?"})))) .
 #					CGI::Tr({}, CGI::td({}, "Delete&nbsp;it?" . CGI::input({type => "checkbox", name => "deleteProblem", value => $problemID}))) .
 					($forOneUser ? "" : CGI::Tr({}, CGI::td({}, CGI::checkbox({name => "markCorrect", value => $problemID, label => "Mark Correct?"})))) .
 				CGI::end_table(),
-				$self->FieldTable($userToShow, $setID, $problemID, $GlobalProblems{$problemID}, $UserProblems{$problemID}),
+				$self->FieldTable($userToShow, $setID, $problemID, $GlobalProblems{$problemID}, $problemToShow, $isGatewaySet),
 # A comprehensive list of problems is just TOO big to be handled well
 #				comboBox({
 #					name => "set.$setID.$problemID",
@@ -1177,7 +2103,7 @@
 					$setID,
 					$problemID,
 					$GlobalProblems{$problemID}, # pass previously fetched global record to FieldHTML --sam
-					$UserProblems{$problemID}, # pass previously fetched user record to FieldHTML --sam
+					$problemToShow, # pass previously fetched user record to FieldHTML --sam
 					"source_file"
 				)) .
 			        	CGI::br() . 
@@ -1189,27 +2115,44 @@
 			]));
 		}
 
+          
+# print final lines
 		print CGI::end_table();
 		print CGI::checkbox({
-				  label=> "Force problems to be numbered consecutively from one",
-				  name=>"force_renumber", value=>"1"}),
-
-		  CGI::br();
-		print CGI::input({type=>"submit", name=>"submit_changes", value=>"Save Changes"});
-		print CGI::input({type=>"submit", name=>"handle_numbers", value=>"Reorder problems only"}) . "(Any unsaved changes will be lost.)";
-		print CGI::p(<<HERE);
+				  label=> "Force problems to be numbered consecutively from one (always done when reordering problems)",
+				  name=>"force_renumber", value=>"1"});
+		print CGI::p(<<EOF);
 Any time problem numbers are intentionally changed, the problems will
 always be renumbered consecutively, starting from one.  When deleting
 problems, gaps will be left in the numbering unless the box above is
 checked.
-HERE
-                print CGI::p("It is before the open date.  You probably want to renumber the problems if you are deleting some from the middle.") if ($setRecord->open_date>time());
-		print CGI::p("When changing problem numbers, we will move 
- the problem to be ", CGI::em("before"), " the chosen number.");
+EOF
+        print CGI::p("It is before the open date.  You probably want to renumber the problems if you are deleting some from the middle.") if ($setRecord->open_date>time());
+		print CGI::p("When changing problem numbers, we will move the problem to be ". CGI::em("before"). " the chosen number.");
 
 	} else {
 		print CGI::p(CGI::b("This set doesn't contain any problems yet."));
 	}
+	# always allow one to add a new problem, unless we're editing a set version
+	if ( ! $editingSetVersion ) {
+		print 	CGI::checkbox({ label=> "Add",
+					name=>"add_blank_problem", value=>"1"}
+			),CGI::input({
+					name=>"add_n_problems",
+					size=>2,
+					value=>1 },
+					"blank problem template(s) to end of homework set"
+			);
+	}
+	print CGI::br(),CGI::br(),
+		CGI::input({type=>"submit", name=>"submit_changes", value=>"Save Changes"}),
+		CGI::input({type=>"submit", name=>"handle_numbers", value=>"Reorder problems only"}),
+			"(Any unsaved changes will be lost.)";
+
+	#my $editNewProblemPage = $urlpath->new(type => 'instructor_problem_editor_withset_withproblem', args => { courseID => $courseID, setID => $setID, problemID =>'new_problem'    });
+    #my $editNewProblemLink = $self->systemLink($editNewProblemPage, params => { make_local_copy => 1, file_type => 'blank_problem'  });
+    # This next feature isn't fully supported and is causing problems.  Remove for now.  #FIXME
+	#print CGI::p( CGI::a({href=>$editNewProblemLink},'Edit'). ' a new blank problem');
 
 	print CGI::end_form();