Replace the missing delete-cascade triggers with deletes in code

WebRoot/js/uwpr.scheduler.js

@@ -265,11 +265,14 @@
                          linksdiv += "<span id='project_detail_tooltip_delete_"+event.id+"' style='color:red; font-size:8pt; font-weigt:bold; text-decoration:underline; cursor:pointer'>[Delete]</span>";
                          linksdiv += "&nbsp;&nbsp;&nbsp;";
                          linksdiv += "<span id='project_detail_tooltip_edit_"+event.id+"' style='color:red; font-size:8pt; font-weigt:bold; text-decoration:underline; cursor:pointer'>[Edit Dates & Operator]</span>";
+                         linksdiv += "</div>";
+
+                         // Edit Project & Payment Method also acts on the selected blocks, so hide it too
+                         // when none can be selected (every block is billed).
+                         linksdiv += '<div style="margin-top: 7px;">';
+                         linksdiv += "<span id='project_detail_tooltip_edit_payment_"+event.id+"' style='color:red; font-size:8pt; font-weigt:bold; text-decoration:underline; cursor:pointer'>[Edit Project & Payment Method]</span>";
                          linksdiv += "</div>";
 					 }
-                     linksdiv += '<div style="margin-top: 7px;">';
-                     linksdiv += "<span id='project_detail_tooltip_edit_payment_"+event.id+"' style='color:red; font-size:8pt; font-weigt:bold; text-decoration:underline; cursor:pointer'>[Edit Project & Payment Method]</span>";
-                     linksdiv += "</div>";
 
                      linksdiv += "</div>"
 

src/PRMessageResources.properties

@@ -119,7 +119,7 @@ error.project.invalidgroup=Somehow, you have sent a group that is not a valid gr
 error.project.noabstract=You must supply an abstract for this project.
 error.project.longAbstract=Abstract cannot exceed 500 words.
 error.project.invalid.progress=Progress report should contain at least 20 words.
-error.project.hasinstrumenttime=This project cannot be deleted because instrument time has been scheduled for it.  Only projects with no scheduled instrument time can be deleted.
+error.project.hasinstrumenttime=This project cannot be deleted because instrument time is recorded for it.  Only projects with no instrument time, scheduled or cancelled, can be deleted.
 error.project.archivedpayment=This project is archived.  Unarchive it before adding or deleting payment methods.
 error.project.archivedinstrumenttime=This project is archived.  Unarchive it before changing instrument time.
 error.project.archivefailed.one=The project could not be updated and was left unchanged.
@@ -173,12 +173,14 @@ error.costcenter.delete=Error deleting: {0}
 error.costcenter.edit=Error editing: {0}
 error.costcenter.export=Error exporting: {0}
 error.costcenter.invalidaccess=Invalid access: {0}
+error.costcenter.notallowed={0}
 
 error.payment.infoincomplete=Incomplete information: {0}
 error.payment.invalidid=Invalid ID: {0}
 error.payment.invalidaccess=Invalid access: {0}
 error.payment.load=Error loading data: {0}
 error.payment.save=Error saving. {0}
+error.payment.delete=This payment method could not be deleted. Please contact UWPR staff and quote reference {0} so they can resolve it.
 
 error.scheduler.invalidid=Invalid ID: {0}
 error.scheduler.save=Error saving. {0}

src/org/uwpr/costcenter/InvoiceBlockCreator.java

@@ -36,29 +36,42 @@ public InvoiceBlockCreator (Invoice invoice) {
 	@Override
 	public void blockExported(UsageBlockBase block) throws BillingInformationExporterException
 	{
-		InvoiceInstrumentUsage oldSavedBlock = null;
+		List<InvoiceInstrumentUsage> allRows;
 		try {
-			oldSavedBlock = invoiceBlockDao.getInvoiceBlock(block.getID());
+			allRows = invoiceBlockDao.getAllInvoiceRowsForUsage(block.getID());
 		}
 		catch(SQLException e) {
 			throw new BillingInformationExporterException("Error getting results from invoiceInstrumentUsage table.", e);
 		}
 
-		// If there is already an entry in the table for this block it means this block
-		// has already been included in an invoice.  If the invoice ID we have been given
-		// is different from the one associated with this block it means that this block
-		// is being included in multiple invoices.  This should never happen
-		if(oldSavedBlock != null) {
-			if(oldSavedBlock.getInvoiceId() != invoice.getId()) {
-				throw new BillingInformationExporterException("Usage block with ID "+block.getID()+" is already part of another invoice");
+		// A block with any invoiceInstrumentUsage row is already invoiced.  A row for this invoice means the
+		// block is already on it (a re-export) -- skip it so saveBlocks does not add a duplicate.  A row for
+		// any other invoice means the block is already invoiced elsewhere, or the row is an orphan left by a
+		// deleted invoice -- refuse rather than invoice over it.
+		boolean alreadyOnThisInvoice = false;
+		List<String> otherRows = new ArrayList<>();
+		for(InvoiceInstrumentUsage row: allRows) {
+			if(row.getInvoiceId() == invoice.getId()) {
+				alreadyOnThisInvoice = true;
 			}
+			else {
+				otherRows.add("id " + row.getId() + " (invoice " + row.getInvoiceId() + ")");
+			}
+		}
+		if(!otherRows.isEmpty()) {
+			throw new BillingInformationExporterException("Usage block " + block.getID()
+					+ " already has an invoiceInstrumentUsage row for another invoice, " + otherRows
+					+ ".  Resolve it before invoicing this block.");
 		}
 
-		// Add to blocks that will be invoiced
-		InvoiceInstrumentUsage invoiceBlock = new InvoiceInstrumentUsage();
-		invoiceBlock.setInvoiceId(invoice.getId());
-		invoiceBlock.setInstrumentUsageId(block.getID());
-		invoicedBlocks.add(invoiceBlock);
+		// Add to blocks that will be invoiced, unless the block already has a row for this invoice, which
+		// would make saveBlocks insert a duplicate.
+		if(!alreadyOnThisInvoice) {
+			InvoiceInstrumentUsage invoiceBlock = new InvoiceInstrumentUsage();
+			invoiceBlock.setInvoiceId(invoice.getId());
+			invoiceBlock.setInstrumentUsageId(block.getID());
+			invoicedBlocks.add(invoiceBlock);
+		}
 	}
 
 	public void updateBlock(UsageBlockBase block) throws BillingInformationExporterException
@@ -81,8 +94,7 @@ else if(block.getStartDate().before(invoice.getBillStartDate()))
 			// This SHOULD NOT happen, unless blocks in the previous billing cycle were not invoiced.
 			try
 			{
-				InvoiceInstrumentUsage invoicedBlock = InvoiceInstrumentUsageDAO.getInstance().getInvoiceBlock(block.getID());
-				if(invoicedBlock != null)
+				if(InvoiceInstrumentUsageDAO.getInstance().isBlockInvoiced(block.getID()))
 				{
 					throw new BillingInformationExporterException("Cannot split block.  It has already been invoiced. " + block.toString());
 				}

src/org/uwpr/costcenter/InvoiceDAO.java

@@ -93,17 +93,33 @@ public Invoice getInvoice (Date startDate, Date endDate) throws SQLException {
 	}
 
 	public void delete(Invoice invoice) throws SQLException {
-		
+
 		String sql = "DELETE FROM invoice WHERE id="+invoice.getId();
 		Connection conn = null;
 		Statement stmt = null;
-		
+
 		try {
 			conn = DBConnectionManager.getMainDbConnection();
+			conn.setAutoCommit(false);
+
+			// Delete the invoice's usage links, then the invoice, on one connection.  No trigger
+			// removes the links, and a link left behind reports its block as billed.
+			// invoiceInstrumentUsage is InnoDB, so a failure on the invoice delete rolls back the
+			// link delete and leaves the invoice re-deletable.  invoice is MyISAM and commits on its
+			// own, so a commit failure after it still strands the links.
+			InvoiceInstrumentUsageDAO.getInstance().deleteBlocksForInvoice(conn, invoice.getId());
+
 			stmt = conn.createStatement();
 			stmt.execute(sql);
+
+			conn.commit();
+		}
+		catch(SQLException e) {
+			if(conn != null) try {conn.rollback();} catch(SQLException ignored){}
+			throw e;
 		}
 		finally {
+			if(conn != null) try {conn.setAutoCommit(true);} catch(SQLException ignored){}
 			if(conn != null) try {conn.close();} catch(SQLException e){}
 			if(stmt != null) try {stmt.close();} catch(SQLException e){}
 		}

src/org/uwpr/costcenter/InvoiceInstrumentUsageDAO.java

@@ -10,6 +10,7 @@
 import org.yeastrc.db.DBConnectionManager;
 
 import java.sql.*;
+import java.util.ArrayList;
 import java.util.List;
 
 /**
@@ -45,33 +46,93 @@ public void saveBlocks(Connection conn, List<InvoiceInstrumentUsage> invoiceBloc
 		}
 	}
 
-	public InvoiceInstrumentUsage getInvoiceBlock (int instrumentUsageId) throws SQLException {
-
-		String sql = "SELECT * FROM invoiceInstrumentUsage WHERE instrumentUsageID="+instrumentUsageId;
+	/**
+	 * True if the block has any invoiceInstrumentUsage row.  Any row means the block is invoiced -- a live
+	 * invoice, or an orphan the pre-deploy cleanup was meant to remove -- so callers refuse to edit or
+	 * delete it either way.
+	 */
+	public boolean isBlockInvoiced (int instrumentUsageId) throws SQLException {
+
+		String sql = "SELECT 1 FROM invoiceInstrumentUsage WHERE instrumentUsageID = ? LIMIT 1";
 		Connection conn = null;
-		Statement stmt = null;
+		PreparedStatement stmt = null;
 		ResultSet rs = null;
-		
+
 		try {
 			conn = DBConnectionManager.getMainDbConnection();
-			stmt = conn.createStatement();
-			rs = stmt.executeQuery(sql);
-
-			if(rs.next())
-			{
-				InvoiceInstrumentUsage invoiceBlock = new InvoiceInstrumentUsage();
-				invoiceBlock.setId(rs.getInt("id"));
-				invoiceBlock.setInvoiceId(rs.getInt("invoiceID"));
-				invoiceBlock.setInstrumentUsageId(rs.getInt("instrumentUsageID"));
-				return invoiceBlock;
+			stmt = conn.prepareStatement(sql);
+			stmt.setInt(1, instrumentUsageId);
+			rs = stmt.executeQuery();
+			return rs.next();
+		}
+		finally {
+			if(conn != null) try {conn.close();} catch(SQLException e){}
+			if(stmt != null) try {stmt.close();} catch(SQLException e){}
+			if(rs != null) try {rs.close();} catch(SQLException e){}
+		}
+	}
+
+	/**
+	 * Returns every invoiceInstrumentUsage row for the block.  blockExported uses this to skip a block
+	 * already on the invoice being built (a re-export) and to refuse one linked to any other invoice,
+	 * whether that invoice still exists or is an orphan left by a deleted one.
+	 */
+	public List<InvoiceInstrumentUsage> getAllInvoiceRowsForUsage (int instrumentUsageId) throws SQLException {
+
+		String sql = "SELECT id, invoiceID, instrumentUsageID FROM invoiceInstrumentUsage WHERE instrumentUsageID = ?";
+		Connection conn = null;
+		PreparedStatement stmt = null;
+		ResultSet rs = null;
+		List<InvoiceInstrumentUsage> rows = new ArrayList<>();
+
+		try {
+			conn = DBConnectionManager.getMainDbConnection();
+			stmt = conn.prepareStatement(sql);
+			stmt.setInt(1, instrumentUsageId);
+			rs = stmt.executeQuery();
+			while(rs.next()) {
+				InvoiceInstrumentUsage row = new InvoiceInstrumentUsage();
+				row.setId(rs.getInt("id"));
+				row.setInvoiceId(rs.getInt("invoiceID"));
+				row.setInstrumentUsageId(rs.getInt("instrumentUsageID"));
+				rows.add(row);
 			}
 		}
 		finally {
 			if(conn != null) try {conn.close();} catch(SQLException e){}
 			if(stmt != null) try {stmt.close();} catch(SQLException e){}
 			if(rs != null) try {rs.close();} catch(SQLException e){}
 		}
-
-		return null;
+		return rows;
+	}
+
+	public void deleteBlocksForInvoice (Connection conn, int invoiceId) throws SQLException {
+
+		String sql = "DELETE FROM invoiceInstrumentUsage WHERE invoiceID = ?";
+		PreparedStatement stmt = null;
+
+		try {
+			stmt = conn.prepareStatement(sql);
+			stmt.setInt(1, invoiceId);
+			stmt.executeUpdate();
+		}
+		finally {
+			if(stmt != null) try {stmt.close();} catch(SQLException e){}
+		}
+	}
+
+	public void deleteBlocksForUsage (Connection conn, int instrumentUsageId) throws SQLException {
+
+		String sql = "DELETE FROM invoiceInstrumentUsage WHERE instrumentUsageID = ?";
+		PreparedStatement stmt = null;
+
+		try {
+			stmt = conn.prepareStatement(sql);
+			stmt.setInt(1, instrumentUsageId);
+			stmt.executeUpdate();
+		}
+		finally {
+			if(stmt != null) try {stmt.close();} catch(SQLException e){}
+		}
 	}
 }

src/org/uwpr/instrumentlog/InstrumentUsageDAO.java

@@ -347,15 +347,6 @@ public int getUsageBlockCountForProject(int projectId) throws SQLException {
 		return getUsageBlockCountForProject(projectId, false, false);
 	}
 
-	/**
-	 * Counts the blocks currently scheduled for the project.  Blocks cancelled before
-	 * 10.28.2022 carry deleted=1 and are excluded.
-	 */
-	public int getScheduledUsageBlockCountForProject(int projectId) throws SQLException {
-
-		return getUsageBlockCountForProject(projectId, true, false);
-	}
-
 	/**
 	 * Counts the blocks scheduled for the project that have not ended yet.  Cancelled blocks
 	 * are excluded.
@@ -397,9 +388,6 @@ private int getUsageBlockCountForProject(int projectId, boolean scheduledOnly, b
 
 	public void purge(UsageBlockBase block, Researcher researcher) throws SQLException {
 
-		// NOTE: There is a trigger on instrumentUsage table that will 
-		//       delete all entries in the instrumentUsagePayment where instrumentUsageID is equal to 
-		//       the given usageId
 		Connection conn = null;
 
 		try {
@@ -422,9 +410,11 @@ private void delete(Connection conn, List<UsageBlockBase> blocks, Researcher res
 		{
 			return;
 		}
-		// NOTE: There is a trigger on instrumentUsage table that will
-		//       delete all entries in the instrumentUsagePayment where instrumentUsageID is equal to
-		//       the given usageId
+
+		// No trigger deletes a usage block's child rows, so delete them here on the caller's
+		// connection, each block's children before the block itself.
+		InstrumentUsagePaymentDAO paymentDao = InstrumentUsagePaymentDAO.getInstance();
+		InvoiceInstrumentUsageDAO invoiceUsageDao = InvoiceInstrumentUsageDAO.getInstance();
 		PreparedStatement stmt = null;
 		String sql = "DELETE FROM instrumentUsage WHERE id=?";
 
@@ -436,11 +426,15 @@ private void delete(Connection conn, List<UsageBlockBase> blocks, Researcher res
 			for(UsageBlockBase block: blocks)
 			{
 				log.info("Deleting usage block ID "+block.getID());
+
+				paymentDao.deletePaymentsForUsage(conn, block.getID());
+				invoiceUsageDao.deleteBlocksForUsage(conn, block.getID());
+
 				stmt.setInt(1, block.getID());
 				stmt.executeUpdate();
 
-				message = message == null ? "" : message + ": ";
-				logDao.logSignupPurged(conn, block, researcher.getID(), message + block.toString());
+				String logMessage = message == null ? "" : message + ": ";
+				logDao.logSignupPurged(conn, block, researcher.getID(), logMessage + block.toString());
 			}
 
 		} finally {

src/org/uwpr/instrumentlog/InstrumentUsagePaymentDAO.java

@@ -126,8 +126,12 @@ private Connection getConnection() throws SQLException {
     }
 
     public boolean hasInstrumentUsageForPayment(int paymentMethodId) throws SQLException {
-
-		String sql = "SELECT count(*) FROM instrumentUsagePayment WHERE paymentMethodID = "+paymentMethodId;
+
+		// Inner-join instrumentUsage so an instrumentUsagePayment row whose usage block no longer exists
+		// (an orphan) does not make the payment method look in use and stop it being deleted or edited.
+		String sql = "SELECT count(*) FROM instrumentUsagePayment iup"
+				+ " INNER JOIN instrumentUsage iu ON iu.id = iup.instrumentUsageID"
+				+ " WHERE iup.paymentMethodID = "+paymentMethodId;
 		Connection conn = null;
 		Statement stmt = null;
 		ResultSet rs = null;
@@ -151,29 +155,46 @@ public boolean hasInstrumentUsageForPayment(int paymentMethodId) throws SQLExcep
 		return false;
 	}
 
-	public void deletePaymentsForUsage (int instrumentUsageId) throws SQLException {
+	public void deletePaymentsForUsage (Connection conn, int instrumentUsageId) throws SQLException {
 
-		Connection conn = null;
+		String sql = "DELETE FROM instrumentUsagePayment WHERE instrumentUsageID = ?";
+		PreparedStatement stmt = null;
 
 		try {
-			deletePaymentsForUsage(conn, instrumentUsageId);
+			stmt = conn.prepareStatement(sql);
+			stmt.setInt(1, instrumentUsageId);
+			stmt.executeUpdate();
 		}
 		finally {
-			if(conn != null) try {conn.close();} catch(SQLException e){}
+			if(stmt != null) try {stmt.close();} catch(SQLException e){}
 		}
 	}
 
-	public void deletePaymentsForUsage (Connection conn, int instrumentUsageId) throws SQLException {
+	/**
+	 * Returns the instrumentUsageID of every instrumentUsagePayment row for the payment method.
+	 * DeletePaymentMethodAction refuses to delete a method with live usage, so a non-empty result means
+	 * orphaned rows whose usage block was purged.  deletePaymentMethod uses this to refuse the delete
+	 * and name the rows rather than delete them silently.
+	 */
+	public List<Integer> getUsageIdsForPaymentMethod (Connection conn, int paymentMethodId) throws SQLException {
 
-		String sql = "DELETE FROM instrumentUsagePayment where instrumentUsageID="+instrumentUsageId;
+		String sql = "SELECT instrumentUsageID FROM instrumentUsagePayment WHERE paymentMethodID = ?";
 		PreparedStatement stmt = null;
+		ResultSet rs = null;
+		List<Integer> usageIds = new ArrayList<>();
 
 		try {
 			stmt = conn.prepareStatement(sql);
-			stmt.executeUpdate();
+			stmt.setInt(1, paymentMethodId);
+			rs = stmt.executeQuery();
+			while(rs.next()) {
+				usageIds.add(rs.getInt("instrumentUsageID"));
+			}
 		}
 		finally {
+			if(rs != null) try {rs.close();} catch(SQLException e){}
 			if(stmt != null) try {stmt.close();} catch(SQLException e){}
 		}
+		return usageIds;
 	}
 }