From b45ffe48b7bf19b74c86c9ecd303bf07d5397362 Mon Sep 17 00:00:00 2001 From: Penar Musaraj Date: Thu, 16 Feb 2023 21:16:48 -0500 Subject: [PATCH] FIX: Allow headings to have the same label (#46) Fixes an issue where multiple headings with the same value would remove columns from displaying. --- .../discourse-table-builder/lib/utilities.js | 21 +++-- .../components/spreadsheet-editor.js | 9 ++- test/fixtures/md-table-fixture.js | 1 - .../md-table-special-chars-fixture.js | 1 - test/fixtures/md-table.js | 3 + test/unit/lib/utilities-test.js | 81 +++++++++++++------ 6 files changed, 77 insertions(+), 39 deletions(-) delete mode 100644 test/fixtures/md-table-fixture.js delete mode 100644 test/fixtures/md-table-special-chars-fixture.js create mode 100644 test/fixtures/md-table.js diff --git a/javascripts/discourse-table-builder/lib/utilities.js b/javascripts/discourse-table-builder/lib/utilities.js index 070b55b..2c270ae 100644 --- a/javascripts/discourse-table-builder/lib/utilities.js +++ b/javascripts/discourse-table-builder/lib/utilities.js @@ -2,26 +2,23 @@ /** * Generate markdown table from an array of objects + * Inspired by https://github.com/Ygilany/array-to-table * - * @see {@link https://github.com/Ygilany/array-to-table|GitHub}: - * - * @param {Array} array Array of objects - * @param {String} columns Optional, table column names, otherwise taken from the keys of the first object + * @param {Array} array Array of objects + * @param {Array} columns Column headings + * @param {String} colPrefix Table column prefix * * @return {String} Markdown table */ -export function arrayToTable(array, columns) { +export function arrayToTable(array, cols, colPrefix = "col") { var table = ""; - // Generate column list - var cols = columns ? columns.split(",") : Object.keys(array[0]); - // Generate table headers table += "|"; table += cols.join(" | "); table += "|\r\n|"; - // Generate table header seperator + // Generate table header separator table += cols .map(function () { return "---"; @@ -32,15 +29,17 @@ export function arrayToTable(array, columns) { // Generate table body array.forEach(function (item) { table += "|"; + table += cols - .map(function (key) { - return String(item[key] || ""); + .map(function (_key, index) { + return String(item[`${colPrefix}${index}`] || ""); }) .join(" | ") + "|\r\n"; }); // Return table + console.log(table); return table; } diff --git a/javascripts/discourse/components/spreadsheet-editor.js b/javascripts/discourse/components/spreadsheet-editor.js index 164b189..475a00a 100644 --- a/javascripts/discourse/components/spreadsheet-editor.js +++ b/javascripts/discourse/components/spreadsheet-editor.js @@ -5,6 +5,7 @@ import { findTableRegex, tokenRange, } from "../discourse-table-builder/lib/utilities"; + import Component from "@glimmer/component"; import { ajax } from "discourse/lib/ajax"; import { popupAjaxError } from "discourse/lib/ajax-error"; @@ -12,6 +13,7 @@ import I18n from "I18n"; import { schedule } from "@ember/runloop"; import { tracked } from "@glimmer/tracking"; import { localeMapping } from "../discourse-table-builder/lib/locale-mapping"; + export default class SpreadsheetEditor extends Component { @tracked showEditReason = false; @tracked loading = null; @@ -244,10 +246,13 @@ export default class SpreadsheetEditor extends Component { data.forEach((row) => { const result = {}; - headers.forEach((key, index) => (result[key] = row[index])); + headers.forEach((_key, index) => { + const columnKey = `col${index}`; + return (result[columnKey] = row[index]); + }); table.push(result); }); - return arrayToTable(table); + return arrayToTable(table, headers); } } diff --git a/test/fixtures/md-table-fixture.js b/test/fixtures/md-table-fixture.js deleted file mode 100644 index 0e554f9..0000000 --- a/test/fixtures/md-table-fixture.js +++ /dev/null @@ -1 +0,0 @@ -export default `|Make | Model | Year|\r\n|--- | --- | ---|\r\n|Toyota | Supra | 1998|\r\n|Nissan | Skyline | 1999|\r\n|Honda | S2000 | 2001|\r\n`; diff --git a/test/fixtures/md-table-special-chars-fixture.js b/test/fixtures/md-table-special-chars-fixture.js deleted file mode 100644 index 55766a1..0000000 --- a/test/fixtures/md-table-special-chars-fixture.js +++ /dev/null @@ -1 +0,0 @@ -export default `|Make | Model | Price|\r\n|--- | --- | ---|\r\n|Toyota | Supra | $50,000|\r\n| | Celica | $20,000|\r\n|Nissan | GTR | $80,000|\r\n`; diff --git a/test/fixtures/md-table.js b/test/fixtures/md-table.js new file mode 100644 index 0000000..cf410f5 --- /dev/null +++ b/test/fixtures/md-table.js @@ -0,0 +1,3 @@ +export const mdTable = `|Make | Model | Year|\r\n|--- | --- | ---|\r\n|Toyota | Supra | 1998|\r\n|Nissan | Skyline | 1999|\r\n|Honda | S2000 | 2001|\r\n`; +export const mdTableSpecialChars = `|Make | Model | Price|\r\n|--- | --- | ---|\r\n|Toyota | Supra | $50,000|\r\n| | Celica | $20,000|\r\n|Nissan | GTR | $80,000|\r\n`; +export const mdTableNonUniqueHeadings = `|col1 | col2 | col1|\r\n|--- | --- | ---|\r\n|Col A | Col B | Col C|\r\n`; diff --git a/test/unit/lib/utilities-test.js b/test/unit/lib/utilities-test.js index c6f7ca8..bcb8a90 100644 --- a/test/unit/lib/utilities-test.js +++ b/test/unit/lib/utilities-test.js @@ -1,7 +1,11 @@ import { discourseModule } from "discourse/tests/helpers/qunit-helpers"; import { test } from "qunit"; -import mdTableFixture from "../../fixtures/md-table-fixture"; -import mdTableSpecialCharsFixture from "../../fixtures/md-table-special-chars-fixture"; +import { + mdTable, + mdTableNonUniqueHeadings, + mdTableSpecialChars, +} from "../../fixtures/md-table"; + import { arrayToTable, findTableRegex, @@ -11,51 +15,80 @@ discourseModule("Unit | Utilities", function () { test("arrayToTable", function (assert) { const tableData = [ { - Make: "Toyota", - Model: "Supra", - Year: "1998", + col0: "Toyota", + col1: "Supra", + col2: "1998", }, { - Make: "Nissan", - Model: "Skyline", - Year: "1999", + col0: "Nissan", + col1: "Skyline", + col2: "1999", }, { - Make: "Honda", - Model: "S2000", - Year: "2001", + col0: "Honda", + col1: "S2000", + col2: "2001", }, ]; assert.strictEqual( - arrayToTable(tableData), - mdTableFixture, + arrayToTable(tableData, ["Make", "Model", "Year"]), + mdTable, "it creates a markdown table from an array of objects (with headers as keys)" ); const specialCharsTableData = [ { - Make: "Toyota", - Model: "Supra", - Price: "$50,000", + col0: "Toyota", + col1: "Supra", + col2: "$50,000", }, { - Make: "", - Model: "Celica", - Price: "$20,000", + col0: "", + col1: "Celica", + col2: "$20,000", }, { - Make: "Nissan", - Model: "GTR", - Price: "$80,000", + col0: "Nissan", + col1: "GTR", + col2: "$80,000", }, ]; assert.strictEqual( - arrayToTable(specialCharsTableData), - mdTableSpecialCharsFixture, + arrayToTable(specialCharsTableData, ["Make", "Model", "Price"]), + mdTableSpecialChars, "it creates a markdown table with special characters in correct alignment" ); + + const nonUniqueColumns = ["col1", "col2", "col1"]; + + assert.strictEqual( + arrayToTable( + [{ col0: "Col A", col1: "Col B", col2: "Col C" }], + nonUniqueColumns + ), + mdTableNonUniqueHeadings, + "it does not suppress a column if heading is the same as another column" + ); + }); + test("arrayToTable with custom column prefix", function (assert) { + const tableData = [ + { + A0: "hey", + A1: "you", + }, + { + A0: "over", + A1: "there", + }, + ]; + + assert.strictEqual( + arrayToTable(tableData, ["Col 1", "Col 2"], "A"), + `|Col 1 | Col 2|\r\n|--- | ---|\r\n|hey | you|\r\n|over | there|\r\n`, + "it works" + ); }); test("findTableRegex", function (assert) {