1

I am building a Google Sheet to act as a workflow of sorts, where each sheet is a stage of the work and upon changing a checkbox Boolean to True the row is moved to the next stage. I also have it set to reverse this process by changing the same Boolean to False.

This appears to be working as intended, and I can move any row to the next stage or send it back. The only downside is that it takes between 1.8s - 3.2s for this operation to take place. This wouldn't be an issue except that if I change one checkbox to True and then, before it has completed the operation, change a second row's checkbox to True, it doesn't run for that second instance because it is still trying to complete the first one. You can imagine that waiting 3 seconds between each operation is not ideal.

I assume that while my script works, it is poorly optimized with a lot of hardcoded variables. I have only done a little work with apps script and am learning, so I am looking as much for understanding and guidance as I am to fixing this issue. I appreciate any help offered, it will help me learn how to tackle the next problem!

Here is my code, if providing anything else is helpful just let me know:

Also, here is a link to a Copy spreadsheet in case you wanted to test what I have so far: https://docs.google.com/spreadsheets/d/1CRJxKhS26nD8Gqd0ozBYSYee7HgypnXcFbuCRanAwVA/edit?usp=sharing

function onEdit(event) {
  var ss = SpreadsheetApp.getActiveSpreadsheet();
  var s = event.source.getActiveSheet();
  var r = event.source.getActiveRange();

  // select target sheet based on active sheet
  // ps = prior sheet, as = active sheet, ts = target sheet
  // selects trigger column and back column
  // tc = trigger column, bc = back column 
  if(s.getName() == "Prospects") {
    var as = "Prospects";
    var ts = "Pricing";
    var tc = 11;
    var bc = 200;
    } else if(s.getName() == "Pricing") { 
      var ps = "Prospects";
      var as = "Pricing";
      var ts = "Onboarding";
      var tc = 34;
      var bc = 11;
      } else if(s.getName() == "Onboarding") { 
        var ps = "Pricing";
        var as = "Onboarding"; 
        var ts = "DASH";
        var tc = 45;
        var bc = 34;
        }
  
  // get first empty row of target sheet
  function getFirstEmptyRowA() {
  var spr = ss.getSheetByName(ts);
  var column = spr.getRange('A:A');
  var values = column.getValues(); // get all data in one call
  var ct = 0;
  while ( values[ct] && values[ct][0] != "" ) {
    ct++;
  }
  return (ct);
}

  // get first empty row of prior sheet
  function getFirstEmptyRowB() {
  var spr = ss.getSheetByName(ps);
  var column = spr.getRange('A:A');
  var values = column.getValues(); // get all data in one call
  var cy = 0;
  while ( values[cy] && values[cy][0] != "" ) {
    cy++;
  }
  return (cy);
}

  if(r.getColumn() == tc && r.getValue() == true) {
    var row = r.getRow();
    var numColumns = s.getLastColumn();
    var targetSheet = ss.getSheetByName(ts);
    var targetLine = getFirstEmptyRowA();
    var target = targetSheet.getRange(getFirstEmptyRowA() + 1, 1);
    s.getRange(row, 1, 1, numColumns).moveTo(target);
    s.deleteRow(row);
  } else if(r.getColumn() == bc && r.getValue() == false) {
    var row = r.getRow();
    var numColumns = s.getLastColumn();
    var targetSheet = ss.getSheetByName(ps);
    var targetLine = getFirstEmptyRowB();
    var target = targetSheet.getRange(getFirstEmptyRowB() + 1, 1);
    s.getRange(row, 1, 1, numColumns).moveTo(target);
    s.deleteRow(row);
  }
}
  • Have you looked at "Best practices"? See [tag info page](https://stackoverflow.com/tags/google-apps-script/info) for free resources and more details. – TheMaster Nov 08 '21 at 19:46
  • 1
    @TheMaster I did check that out, but it didn't seem to apply to this situation. I specifically looked at "Optimizing Scripts for Better performance" but to me it seems like those don't apply - since I am only looking at a single row at a time, batching doesn't apply. I am also not calling other services or using any libraries. Maybe I am missing how I might utilize something on that page, but it didn't seem to apply here. – Josh Kreuer Nov 08 '21 at 20:11
  • Could you share a sample spreadsheet? – Nikko J. Nov 08 '21 at 21:06
  • Why do you need `getFirstEmptyRowA`? Wouldn't getLastRow() be sufficient? – TheMaster Nov 08 '21 at 21:13
  • Here is a copy of the project, I believe this should work. if it does not let me know! https://docs.google.com/spreadsheets/d/1CRJxKhS26nD8Gqd0ozBYSYee7HgypnXcFbuCRanAwVA/edit?usp=sharing I am also placing the link in the original question above – Josh Kreuer Nov 08 '21 at 21:37
  • @TheMaster I started with getLastRow(), the issue I had with it is when the row moved it would jump to the very bottom of the page (row 1000) because there are formulas running through some of the cells cascaded down. So the only workaround that has worked so far is to have the getFirstEmptyRow. Maybe I am doing something incorrectly though, and would appreciate any feedback, but that is the rationale with the current approach – Josh Kreuer Nov 08 '21 at 21:39
  • Checkout my answer [here](https://stackoverflow.com/questions/46883862/arrayformula-is-breaking-the-getlastrow-funtion-possible-workarounds/46884012) for fixing array formulas. – TheMaster Nov 08 '21 at 21:41
  • @TheMaster thank you so much for the code below and for that link! I will study both and respond back afterward to let you know how it works. I really appreciate the time and effort thus far and hope it resolves the issues! – Josh Kreuer Nov 08 '21 at 21:45
  • About sharing spreadsheets, do note that [it exposes your email address](https://meta.stackoverflow.com/questions/394304/is-it-ethical-to-ask-for-a-google-sheets-file-when-answering-a-question-even-w). So, use a dummy Google account if you want your email to be private. – TheMaster Nov 08 '21 at 21:47
  • Thank you for the advice! I will get that fixed. – Josh Kreuer Nov 08 '21 at 21:49

2 Answers2

1

Here I updated your code by removing unused and duplicate variables, optimize the use of event object, and use different approach for finding the last row of a single column.

Code:

function onEdit(event) {
  var ss = SpreadsheetApp.getActiveSpreadsheet();
  var r = event.range;
  var s = event.source.getActiveSheet();
  var sheetName = s.getName();
  if (sheetName == "Prospects") {
    var ts = "Pricing";
    var tc = 11;
    var bc = 200;
  } else if (sheetName == "Pricing") {
    var ps = "Prospects";
    var ts = "Onboarding";
    var tc = 34;
    var bc = 11;
  } else if (sheetName == "Onboarding") {
    var ps = "Pricing";
    var ts = "DASH";
    var tc = 45;
    var bc = 34;
  }

  var targetSheet = "";
  if (r.getColumn() == tc && event.value == "TRUE") {
    targetSheet = ts;
  } else if (r.getColumn() == bc && event.value == "FALSE") {
    targetSheet = ps;
  }
  var lastRow = ss.getSheetByName(targetSheet).getRange(1, 1).getDataRegion(SpreadsheetApp.Dimension.ROWS).getLastRow()
  var row = r.getRow();
  var numColumns = s.getLastColumn();
  var target = ss.getSheetByName(targetSheet).getRange(lastRow + 1, 1);
  s.getRange(row, 1, 1, numColumns).moveTo(target);
  s.deleteRow(row);
}

By using this approach, the execution time was reduced to 1-1.5 seconds.

enter image description here

Reference:

Nikko J.
  • 5,319
  • 1
  • 5
  • 14
  • Have you tested this in OP's sheet? I thought checkboxes were the reason for `getLastRow()` going to 1000. Does `getDataRegion()` bypass that to provide the correct last row? – TheMaster Nov 10 '21 at 16:08
  • 1
    @TheMaster - I copied the data from OP's sheet and paste it into my dummy spreadsheet then test it and yes, I was able to get the correct last row of a specific column using getDataRegion(dimension). – Nikko J. Nov 10 '21 at 17:06
  • 1
    @NikkoJ. sorry for the late reply, just finished testing and this is working beautifully! Down to under 1s execution time (average of 0.844s)! – Josh Kreuer Nov 15 '21 at 17:39
0
  • There are many repeated calls to the same function like s.getName() thrice and getFirstEmptyRow() twice in one function. This is a violation of DRY principle and causes unnecessary burden on the script.

  • If getFirstEmptyRow() is not necessary, it can be replaced with .getLastRow()

  • If it is still slow, use CacheService or PropertiesService to save key values like the last available row number in each sheet.

/**
 * @param {GoogleAppsScript.Events.SheetsOnEdit} event
 */
function onEdit(event) {
  const ss = event.source;
  const editedRange = event.range;
  const activeSheet = event.range.getSheet();
  const [editedRow, editedColumn] = [
    editedRange.getRow(),
    editedRange.getColumn(),
  ];
  let priorSheetName,
    targetSheetName,
    triggerColumn,
    backColumn,
    activeSheetName;
  activeSheetName = activeSheet.getName(); //call only once
  if (activeSheetName === 'Prospects') {
    targetSheetName = 'Pricing';
    triggerColumn = 11;
    backColumn = 200;
  } else if (activeSheetName === 'Pricing') {
    priorSheetName = 'Prospects';
    targetSheetName = 'Onboarding';
    triggerColumn = 34;
    backColumn = 11;
  } else if (activeSheetName === 'Onboarding') {
    priorSheetName = 'Pricing';
    targetSheetName = 'DASH';
    triggerColumn = 45;
    backColumn = 34;
  }

  /**
   * @param {GoogleAppsScript.Spreadsheet.Sheet} spr
   * get first empty row of target sheet
   */
  function getFirstEmptyRow(spr) {
    const columnAValues = spr.getRange('A:A');
    const values = columnAValues.getValues(); // get all data in one call
    let ct = 0;
    while (values[ct] && values[ct][0] != '') {
      ct++;
    }
    return ct + 1;
  }
  /**
   * Moves and deletes row in sheet
   * @param {String} sheetName
   */
  function moveAndDelete(sheetName) {
    const numColumns = activeSheet.getLastColumn();
    const targetSheet = ss.getSheetByName(sheetName);
    const targetLine = getFirstEmptyRow(targetSheet);
    const target = targetSheet.getRange(targetLine, 1);
    activeSheet.getRange(editedRow, 1, 1, numColumns).moveTo(target);
    //Not needed activeSheet.deleteRow(editedRow);
  }

  if (editedColumn === triggerColumn && event.value === 'true') {
    moveAndDelete(targetSheetName);
  } else if (editedColumn === backColumn && event.value === 'false') {
    moveAndDelete(priorSheetName);
  }
}
TheMaster
  • 45,448
  • 6
  • 62
  • 85