Skip to content

Commit

Permalink
[FIX] mergeContiguousZones: add test & fix diagonal
Browse files Browse the repository at this point in the history
This commit moves the tests of mergeContiguousZones from Odoo
to o-spreadsheet.

It also fixes a bug where the algorithm was merging zones that were
diagonal to each other.

closes #4332

Task: 3869372
Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
  • Loading branch information
hokolomopo committed Jun 11, 2024
1 parent 907fd17 commit 03dfc9e
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 11 deletions.
19 changes: 8 additions & 11 deletions src/helpers/zones.ts
Original file line number Diff line number Diff line change
Expand Up @@ -609,24 +609,21 @@ export function unionPositionsToZone(positions: Position[]): Zone {
* Check if two zones are contiguous, ie. that they share a border
*/
function areZoneContiguous(zone1: Zone, zone2: Zone) {
const u = union(zone1, zone2);
if (zone1.right + 1 === zone2.left || zone1.left === zone2.right + 1) {
return getZoneHeight(u) <= getZoneHeight(zone1) + getZoneHeight(zone2);
return (
(zone1.top <= zone2.bottom && zone1.top >= zone2.top) ||
(zone2.top <= zone1.bottom && zone2.top >= zone1.top)
);
}
if (zone1.bottom + 1 === zone2.top || zone1.top === zone2.bottom + 1) {
return getZoneWidth(u) <= getZoneWidth(zone1) + getZoneWidth(zone2);
return (
(zone1.left <= zone2.right && zone1.left >= zone2.left) ||
(zone2.left <= zone1.right && zone2.left >= zone1.left)
);
}
return false;
}

function getZoneHeight(zone: Zone) {
return zone.bottom - zone.top + 1;
}

function getZoneWidth(zone: Zone) {
return zone.right - zone.left + 1;
}

/**
* Merge contiguous and overlapping zones that are in the array into bigger zones
*/
Expand Down
42 changes: 42 additions & 0 deletions tests/helpers/zones_helpers.test.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import {
createAdaptedZone,
isZoneValid,
mergeContiguousZones,
overlap,
positions,
toCartesian,
toUnboundedZone,
toZone,
zoneToXc,
} from "../../src/helpers/index";

import { Zone } from "../../src/types";
import { target } from "../test_helpers/helpers";

describe("overlap", () => {
test("one zone above the other", () => {
Expand Down Expand Up @@ -153,3 +157,41 @@ describe("createAdaptedZone", () => {
expect(createAdaptedZone(zone, "both", "RESIZE", [-1, -1])).toEqual(toZone("B2"));
});
});

describe("mergeContiguousZones", () => {
test("mergeContiguousZones: can merge two contiguous zones", () => {
let zones = mergeContiguousZones(target("A1:A6, B1:B6"));
expect(zones.map(zoneToXc)).toEqual(["A1:B6"]);

zones = mergeContiguousZones(target("A1:D1, A2:D2"));
expect(zones.map(zoneToXc)).toEqual(["A1:D2"]);

zones = mergeContiguousZones(target("A1:A6, B2"));
expect(zones.map(zoneToXc)).toEqual(["A1:B6"]);

zones = mergeContiguousZones(target("C1, A2:F2"));
expect(zones.map(zoneToXc)).toEqual(["A1:F2"]);

// Not contiguous
zones = mergeContiguousZones(target("C1, C3"));
expect(zones.map(zoneToXc)).toEqual(["C1", "C3"]);
});

test("mergeContiguousZones: can merge two overlapping zones", () => {
let zones = mergeContiguousZones(target("A1:A6, A1:C4"));
expect(zones.map(zoneToXc)).toEqual(["A1:C6"]);

zones = mergeContiguousZones(target("A1:C6, A1:B5"));
expect(zones.map(zoneToXc)).toEqual(["A1:C6"]);
});

test("mergeContiguousZones: can merge overlapping and contiguous zones", () => {
const zones = mergeContiguousZones(target("A1:A6, A1:C4, A7"));
expect(zones.map(zoneToXc)).toEqual(["A1:C7"]);
});

test("Zones diagonally next to each other are not contiguous", () => {
const zones = mergeContiguousZones(target("A1, B2, C3, A3"));
expect(zones.map(zoneToXc)).toEqual(["A1", "B2", "C3", "A3"]);
});
});

0 comments on commit 03dfc9e

Please sign in to comment.