0
\$\begingroup\$

Background
I am a relatively new developer(> 2 years coding) working as an intern for a smaller company, as part of a now 4 person team. We had two other team members, one our system admin and the other our IT manager, fired less than a month after I was hired on. I was hoping to use our IT manager as a resource for reviewing and talking about code with me, as we do not have any other team members with coding knowledge/experience, however now that he has been removed from his position I wanted to reach out to this community to see if I could collaborate with other like-minded people on the code I am writing.

Code Explanation
When I was first hired, I was tasked with creating an automation for our new user onboarding process. We are currently a Google shop, thus we are using Google Workspace for our enterprise suite. To leverage some of the resources available and reduce time/effort needed to make a web application, I am using a Google Form to collect information from our HR team about new hires. The Google Form responses are linked to a Google Sheet (Form Responses), which is used to house the data about our new employees. Once the data is received by the Google Sheet, I am using a Zapier integration with Google Sheets to detect a new spreadsheet row and alert our IT team via Slack that a new employee needs to be "approved" by our team.(Circumstantially, not all users will need accounts within our organization. Thus the need for "approval" of new users).

I have a Google App Script project tied to the Google Sheet containing the Form Responses that appends a Checkbox to new rows added to the sheet with the following code (If the checkbox is checked, the user is approved and accounts should be created):
Form Response Code

function FormSubmit(e){
  var ss = SpreadsheetApp.getActive(); 
  ss.getSheetByName("Form Responses").appendRow(e.values);
  
  var lastRow = ss.getSheetByName("Form Responses").getLastRow();
  var lastColumn = ss.getSheetByName("Form Responses").getLastColumn();

  var endCell = ss.getSheetByName("Form Responses").getRange(lastRow,lastColumn);
  endCell.insertCheckboxes();
}

From here, a member of our team would open the Form Responses Google Sheet, click the checkbox to "approve" the creation of accounts for the user, and click a button embedded on the Form Responses Google Sheet to automate the creation of their Google Account. When the Google Account is created, the new account will also be added to all of the groups they need to be a member of, along with the Shared Drives they need access to.

Image of button/checkbox

Column Checkbox and Button Image

In order to have the right information needed to create Windows AD and LastPass accounts inside of our organization, the button not only creates the new employees Google account, but also sends formatted data obtained from the Form Responses Google Sheet over to another Google Sheet(New Users) to store the data for future use. Error logging for the automation is stored on a Google Doc, that I am appending the "logs" to after the script runs.

Code for Button

function MoveRows() {
  //Get the values of the current spreadsheet
  var spreadSheet = SpreadsheetApp.getActiveSpreadsheet();
  var sourceSheet = spreadSheet.getSheetByName("Form Responses");
  var sourceRange = sourceSheet.getDataRange();
  var sourceValues = sourceRange.getValues();

   
   //Loops through the rows of the SpreadSheet
   for(i = 1; i < sourceValues.length; i++ )
   {
     //If the user was checked, proceed
     if(sourceValues[i][16] == true){
      //Assign values to Users
      var firstName = sourceValues[i][2];
      var lastName = sourceValues[i][3];
      var userName = firstName.substring(0,1) + lastName;
      var email = firstName + "." + lastName + sourceValues[i][11];
      var streetAddress = "###";
      var city = "###";
      var zipCode = "###";
      var state = "###";
      var country = "###";
      var costCenter = sourceValues[i][8];
      var password = "###";
      var orgUnitPath;
      //If there is no department for our User(user not in Appleton)
      //then their "department" is going to be the location of the store they are working at
      if(sourceValues[i][7] == "###"){
        var department = sourceValues[i][10];
        var location = sourceValues[i][7];
      }else{
        department = location = sourceValues[i][7];
      }

      //Switch used to decide OrgUnit Path, 'Location' column is used to identify which OrgUnit the User belongs to
      switch(location){
        case "Philippines - Listings":
        case "Philippines - Pricing":
        case "Phillipines - OCC/Other":
          orgUnitPath = "/People - Philippines"
          break;
        case "###":
          orgUnitPath = "###"
          break;
        default:
          orgUnitPath = "/People - Retail"
          break;
      }

      //Batch object that contains the new user to be added to our workspace
      var user = {
        "primaryEmail": email,
        "password": password,
        "orgUnitPath": orgUnitPath,
        "organizations": [{
          "costCenter": costCenter,
          "department": department
        }], 
        "name" : {
          "familyName": lastName,
          "givenName": firstName
        }
      }
      
      var errorLog;

      try{
      //API call to add the new user to our Google Workspace
      AdminDirectory.Users.insert(user)
      }catch(error){
        errorLog += '{ERROR Google}: The Google Account was not able to be created: Message ' + error.Message;
      }

      //Switch for assigning role based groups and shared drives
      switch(ssValues[i][9]){
        case "Accounting":
          AdminDirectory.Members.insert(groupMember, "###");
          AdminDirectory.Members.insert(groupMember, "###");
          AdminDirectory.Members.insert(groupMember, "###");
          AdminDirectory.Members.insert(groupMember, "###");
          DriveApp.getFolderById("###").addEditor(email);
          break;
          //...repetitive switch code, can include if necessary 
      }
      //Create a row to hold the new values
      var targetValues = [firstName,lastName,userName,email,streetAddress,city,zipCode,state,country,department,location,costCenter];
      //Get the sheet to move the row to 
      var targetSheet = SpreadsheetApp.openById("###");
      //Append the data to the rows
      targetSheet.appendRow(targetValues); 
     }else{
       continue;
     }
   }

  //Used to grab the Google Doc that is holding the logs for error handling
  var logDoc = Docs.Documents.get("###");
  /*Batch object that is used to make an API call to the Google Docs API
  Object holds the text that is set to be added to the log
  This object is the purpose behind grabbing the Google Doc before posting information to it
  Revision ID is needed to keep all of the current data from the log
  */
  var updateObject={
    "requests":[
      {
        "insertText":{
            "text": "\n" + errorLog.toString(),
            "endOfSegmentLocation":{
              "segmentId": ""
            }
        }
      }
    ],
    "writeControl":{
      "requiredRevisionId":logDoc.revisionId
    }
  }
  //This API call is used to add the batch object to the Google Doc(Onboard Automation Logs)
  Docs.Documents.batchUpdate(updateObject, "###");

  //Delete the rows of data that were moved to "New Users"
  sourceSheet.deleteRows(2,sourceValues.length);
}

Where can I improve my error handling on script/ do I need more error handling? Can I improve on the use of best practice anywhere in this code? Any performance concerns?

I have included a ### everywhere that might be containing some sensitive information about our organization. If there is any information that appears to be missing and is vital for reviewing this code please let me know. If there are too many questions/questions are too vague, please let me know and I will revise accordingly. Any other suggested advice, even if it is not related to the questions I asked, is more than welcomed.

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

You could leverage a more object orient approach to create an instance of a user

const sourceValues = [
    [null, "John", "Doe"],
    [null,"Bill", "Nye"]
]

class User{
    constructor(_,firstName, lastName, ...args ) {
        this.firstName = firstName;
        this.lastName = lastName
    }
    userEmail(){
        return this.firstName + "." + this.lastName // + sourceValues[i][11];
    }
}


userList = sourceValues.map(args=>new User(...args))

or use a a zip like function to map the user info to the name of the column.

const COLUMN_NAMES = ["Unknown", "firstName", "lastName"]

function zip(columnNames, userArr){
  
  return userArr.map(userData=>{
    let out = {}
    for (var i = 0; i < columnNames.length; i++) {
        out = { ...out, [columnNames[i]]: userData[i]   }
   }
    return out
  })

}

zip(COLUMN_NAMES,sourceValues)

Either one of these approaches would make your code much more understandable to another developer.

\$\endgroup\$
2
  • \$\begingroup\$ Awesome, I knew this code could be written in a better way that "makes more sense" to others that program just wasn't quite sure on the direction. This is my first time seeing the zip function, think I am going to try that approach first so I can learn something new in the process. Will most likely end up writing both ways to work on my skills as a whole. Thank you for your advice. \$\endgroup\$
    – RankinJ
    Commented Jul 1, 2022 at 19:38
  • \$\begingroup\$ I first came across zip in python, there is a JavaScript library called d3 that also has a a zip. Zip isn't exclusive to creating a object from 2 arrays but it is a use case. You could combine both methods. inside the constructor of the class call the zip function to set this.userInfo = zip(columns, userArr) \$\endgroup\$ Commented Jul 1, 2022 at 20:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.