fix: ... (spread vs. rest property) variable assignment

[DataDrivenMD]

👀 Please review/merge #363 before this PR

Examples of the problem:

const id = 'ActorA'
const preferredUsername = 'jorge'
const properties = { id: 'ActorB', preferredUsername: 'sven'}

const example0 = { id, preferredUsername, properties }
console.log( JSON.stringify(example0) )
// '{"id":"ActorA","preferredUsername":"jorge","properties":{"id":"ActorB","preferredUsername":"sven"}}'

const example1 = { id, preferredUsername, ...properties }
console.log( JSON.stringify(example1) )
// '{"id":"ActorB","preferredUsername":"sven"}'

const example2 = { id, ...properties, preferredUsername }
console.log( JSON.stringify(example2) )
// '{"id":"ActorB","preferredUsername":"jorge"}'

const example3 = { ...properties, id, preferredUsername }
console.log( JSON.stringify(example3) )
// '{"id":"ActorA","preferredUsername":"jorge"}'

Fix the problem

• Non-breaking fixes to ...spread vs. ...rest destructuring in front-end code

• Added TODO flags to code segments that need to be re-reviewed to ensure that they are working as intended. Specifically, the flags are added to array concatenations that involve the spread operator such as allStatuses = [...allStatuses, ...newStatuses], which would yield unpredictable results when multi-dimensional arrays are involved.

• Fixed several several dozen invocations of improper ...rest property binding that involve HTTP response headers. As previously written, the resulting headers object would omit elements that followed ...cors(), in the assignment expression.

• Fixed additional variable assignments that use ...rest property destructuring. Note: this commit breaks existing tests, which were wrongly PASS as a result of bugs in the underlying code that were not being caught by the tests.

Identify bugs caused by the problem and Fix bugs caused by the problem

• Fix several bugs in reblog (ActivityPub Announce handler) business logic:
  • Fix bug that allowed multiple AP Announce activities for the same remote AP Object
  • Fix bug that allowed AP Announce for APObject with a null payload
  • Fix bug that would create notification when a user re-blogs a post that they authored
  • Fix bug that would call fetch() in an attempt to cache APObject whenever a local user re-blogs a post that they authored
• Fix bugs in reblog logic that would allow reblog + notification for remote objects that were inaccessible by the local instance (i.e. due to connectivity errors or de-federation/blocking by remote instance)
• Fix bugs in several tests that were using the wrong ID type for assertions (e.g. checking for equality of remote APObject id instead of checking for equality of local APObject id)

TL; DR:

Tests that were previously passing but are now breaking rely on assertions that validate hard-coded defaults－ these values don't change when the business logic yields null or undefined values along the way, as was happening in several areas due to variable assignment and object binding using the spread and rest operators (...). Now that the correct syntax is being used and the bugs in the original code has been addressed, the proper use of ... in variable assignment should be a point of emphasis for code reviews moving forward.

References:

https://gist.github.com/yesvods/51af798dd1e7058625f4

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Spread_syntax#copy_an_array

cc @xtuc @dario-piotrowicz

[DataDrivenMD]

NOTE: Manually setting the type to APObject here is technically invalid because not all of the objects that are passed to the method have id, type, and url properties. As none of these properties are accessed in this method, the type-checking doesn't fail within the method. This explains why, despite not having an id or type property, we throw an error during mocking when we "overwrite" these properties by calling the ...properties operation during variable assignment

[DataDrivenMD]

It's unnecessary to set const properties = JSON.parse(row.properties) because row.properties === sanitizedProperties

[DataDrivenMD]

It's unnecessary to set type: row.type and id: new URL(row.id), because row.properties === sanitizedProperties. So, the expansion ...sanitizedProperties should suffice

[DataDrivenMD]

Sorry for the spam. Trying to revert an accidental merge commit that pulled in several upstream merge commits 🤦🏼

[DataDrivenMD]

Closing this PR while I sort out the mess I created for myself. Apologies again for the spam