mirrors/nomulus
1
0
Fork 0
mirror of https://github.com/google/nomulus synced 2025-12-23 14:25:44 +00:00
Code Issues Releases Wiki Activity

Handle bad header names in registrar sheet syncing action (#2404)

Browse Source 
The existing behavior was to ignore bad header names, in a way that was
counter-intuitive as a user of the Google Sheet. If a header name was bad (which
could just be someone accidentally changing it not realizing it needs to
correspond exactly to the name of the field on the Java object), then all of the
data in that column was just silently left as-is and never updated. This led to
gradually worsening sync and offset shift errors over time.

Now, it will write out an error message into every single cell in the bad
column, so it's clear that the column name is wrong and does not correspond to any
actual data in the DB.

BUG=http://b/332336068
This commit is contained in:
Ben McIlwain
2024-04-19 13:59:58 -04:00
committed by GitHub
parent fa6898167b
commit 91615aef54
2 changed files with 12 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
core/src/main/java/google/registry/export/sheet/SheetSynchronizer.java
View File

@@ -38,6 +38,8 @@ class SheetSynchronizer {
private static final String SHEET_NAME = "Registrars";
private static final String INVALID_HEADER_TEXT = "Invalid Sheet column header name";
@Inject Sheets sheetsService;
@Inject SheetSynchronizer() {}
@@ -92,10 +94,12 @@ class SheetSynchronizer {
cellRow.add("");
}
for (int j = 0; j < headers.size(); j++) {
// Look for the value corresponding to the row and header indices in data
String dataField = data.get(i).get(headers.get(j));
// If the cell's header matches a data header, and the values aren't equal, mutate it
if (dataField != null && !cellRow.get(j).toString().equals(dataField)) {
// Look for the value corresponding to the row and header indices in data, otherwise write
// out an error so it's clear to a reader of the Google Sheet that the column header is
// invalid.
String dataField = data.get(i).getOrDefault(headers.get(j), INVALID_HEADER_TEXT);
// If the existing cell value is different than the new data, mutate it.
if (!cellRow.get(j).toString().equals(dataField)) {
mutated = true;
originalVals.get(i).set(j, dataField);
}

8
core/src/test/java/google/registry/export/sheet/SheetSynchronizerTest.java
View File

@@ -114,7 +114,7 @@ class SheetSynchronizerTest {
}
@Test
void testSynchronize_unknownFields_doesntUpdate() throws Exception {
void testSynchronize_unknownFields_writesColumnHeaderErrorIntoCell() throws Exception {
existingSheet.add(createRow("a", "c", "b"));
existingSheet.add(createRow("diffVal1", "sameVal", "diffVal2"));
data = ImmutableList.of(ImmutableMap.of("a", "val1", "b", "val2", "d", "val3"));
@@ -125,7 +125,7 @@ class SheetSynchronizerTest {
BatchUpdateValuesRequest expectedRequest = new BatchUpdateValuesRequest();
List<List<Object>> expectedVals = newArrayList();
expectedVals.add(createRow("val1", "sameVal", "val2"));
expectedVals.add(createRow("val1", "Invalid Sheet column header name", "val2"));
expectedRequest.setData(
newArrayList(new ValueRange().setRange("Registrars!A2").setValues(expectedVals)));
expectedRequest.setValueInputOption("RAW");
@@ -133,7 +133,7 @@ class SheetSynchronizerTest {
}
@Test
void testSynchronize_notFullRow_getsPadded() throws Exception {
void testSynchronize_notFullRow_isHandledCorrectly() throws Exception {
existingSheet.add(createRow("a", "c", "b"));
existingSheet.add(createRow("diffVal1", "diffVal2"));
data = ImmutableList.of(ImmutableMap.of("a", "val1", "b", "paddedVal", "d", "val3"));
@@ -144,7 +144,7 @@ class SheetSynchronizerTest {
BatchUpdateValuesRequest expectedRequest = new BatchUpdateValuesRequest();
List<List<Object>> expectedVals = newArrayList();
expectedVals.add(createRow("val1", "diffVal2", "paddedVal"));
expectedVals.add(createRow("val1", "Invalid Sheet column header name", "paddedVal"));
expectedRequest.setData(
newArrayList(new ValueRange().setRange("Registrars!A2").setValues(expectedVals)));
expectedRequest.setValueInputOption("RAW");