fix!: rights conservation should allow new brands or dropped brands if sum is empty

[katelynsills]

Closes #3033

The rights conservation code previously checked that the brands before and after a proposed allocation were exactly the same. This assumption is wrong, because from a rights allocation perspective, it is ok to include a new brand or drop a brand, as long as the sum is empty.

From the code comments:

  // We cannot assume that all of the brand keys present in
  // leftSumsByBrand are also present in rightSumsByBrand. A empty
  // amount could be introduced or dropped, and this should still be
  // deemed "equal" from the perspective of rights conservation.

  // Thus, we should allow for a brand to be missing from a map, but
  // only if the sum for the brand in the other map is empty.

[erights]

If you union the brands, you can do each comparison only once.

Suggested change

	mapToIterate.keys().forEach(brand => {
	const allBrands = new Set([...mapToIterate.keys(), ...mapToCheck.keys()]);
	allBrands.forEach(brand => {

[katelynsills]

Oo, nice!

[katelynsills]

huh, looking at this method in isolation though, we don't know if the brand is defined in the left or the right. And to make empty, we need an amount from the other map. The code gets messy because it doesn't have a place to stand. Let me see if I can improve this.

[katelynsills]

Ok, pushed up a version that uses allBrands, but is a little more verbose

[erights]

LGTM