groton-school / blackbaud-to-google-lists

Google Workspace Add-on to export/sync Blackbaud lists to Google Sheets
GNU General Public License v3.0
0 stars 1 forks source link

should this be resilient to sheet name changes? #34

Open github-actions[bot] opened 1 year ago

github-actions[bot] commented 1 year ago

https://github.com/groton-school/bb-lists-to-google-sheets/blob/647c55c73bc400111159c0dce88e9ce1db780181/src/Metadata.ts#L35


        return null;
    }
    const sheet = SpreadsheetApp.getActive().getSheetByName(json.sheet);
    // TODO should this be resilient to sheet name changes?
    return sheet.getRange(json.row, json.column, json.numRows, json.numColumns);
}

const get = (key: string, sheet?: GoogleAppsScript.Spreadsheet.Sheet) => {
    sheet = sheet || SpreadsheetApp.getActive().getActiveSheet();
    return g.SpreadsheetApp.DeveloperMetadata.get(sheet, key);
};

export const getList = (
    sheet?: GoogleAppsScript.Spreadsheet.Sheet
): SKY.School.Lists.Metadata => get(LIST, sheet);
export const getRange = (sheet?: GoogleAppsScript.Spreadsheet.Sheet) =>
    rangeFromJSON(get(RANGE, sheet));
export const getLastUpdated = (sheet?: GoogleAppsScript.Spreadsheet.Sheet) =>
    get(LAST_UPDATED, sheet);

const set = (
    key: string,
    sheet: GoogleAppsScript.Spreadsheet.Sheet = null,
    value: any
) => {
    sheet = sheet || SpreadsheetApp.getActive().getActiveSheet();
    return g.SpreadsheetApp.DeveloperMetadata.set(sheet, key, value);
};

export const setList = (
    list: SKY.School.Lists.Metadata,
    sheet?: GoogleAppsScript.Spreadsheet.Sheet
) => set(LIST, sheet, list);
export const setRange = (
    range: GoogleAppsScript.Spreadsheet.Range,
    sheet?: GoogleAppsScript.Spreadsheet.Sheet
) => set(RANGE, sheet, rangeToJSON(range));
export const setLastUpdated = (
    lastUpdated: Date,
    sheet?: GoogleAppsScript.Spreadsheet.Sheet
) => set(LAST_UPDATED, sheet, lastUpdated.toLocaleString());

const remove = (key: string, sheet?: GoogleAppsScript.Spreadsheet.Sheet) => {
    sheet = sheet || SpreadsheetApp.getActive().getActiveSheet();
    return g.SpreadsheetApp.DeveloperMetadata.remove(sheet, key);
};

export const removeList = remove.bind(null, LIST);
export const removeRange = remove.bind(null, RANGE);
battis commented 1 year ago

Yes. Yes, it should.

The current behavior is that when the sheet's name is changed, trying to run an update -- even with valid Developer Metadata connecting the sheet to an Advanced List -- is that you get a Google App Script error trying to getRange() on a null value (the sheet that can't be found, since it's only known by name).

Need to either switch to storing sheet by ID (can't remember right now why I'm not doing that) or simply assuming that the sheet affected is the sheet on which the Developer Metadata is stored (a pretty reasonable assumption, one would think).

battis commented 1 year ago

(Ironically, this turned up in a situation where the Advanced List name had also changed, but because that is tracked by ID, not name, it didn't break.)

battis commented 1 year ago

Yes, use the presence of developer metadata to determine that the sheet should be synced. And then use the location of the comment (which should be stored in developer metadata as well as the range last updated) to identify where that range is now.

Could, perhaps, have two comments, one at TL and one at BR, but not sure what to assume if one of them goes missing entirely.

Perhaps if eiter comments goes missing, fail and require the user to manually select the range to be updated?