Refactor code where an array index is used to access elements from another array.

[latin-panda]

There are places where arrayA index is used to access arrayB in messages tab, relying that both arrays are synchronised. This could be easily breakable.

Example: AddReadStatusService

Analyse Message Tab code to find this places and find out a better reliable way of implementing the feature.

[dianabarsan]

In your example, allDocs will always return results in the exact same order you've requested them.

We use this in a multitude of places throughout the app, and is a feature of both CouchDB and PouchDB.

[dianabarsan]

Are there other places, where we don't "assume" results from the DB are in the same order as our requested objects where you've seen this?

[latin-panda]

I remember seen this 2 times in message tab code, one is AddReadStatusService, the second one will require some investigation, I suggested an analysis of more occurrences as par of the ticket's job :)

It's nice that CouchDB and PouchDB has this feature of returning the records on the exact same order, however I feel uneasy to rely on this, someone can add a middleware altering the sort of records to accomodate a need, and forget about these pieces of code that relies on matching indexes. This problem will possibly be detected during testing, but I still consider worth of refactoring to a "less easy to break and more independent" code 🤔

I also suggest to do a performance comparison between the two; index access will always be faster, but we can come up with other solutions (sort of id-idx mappings, etc..) still performing good enough and trying to avoid nested loops. Interesting also as part of the analysis in the ticket's job. I think this is low priority, nice to have. 🙂

[dianabarsan]

someone can add a middleware altering the sort of records to accomodate a need

It would be pretty terrible if someone did that, I'd consider it as terrible as prototype pollution. Like we don't particularly test for prototype pollution, we probably shouldn't test for middlewares that alter how extremely important APIs function.

thoughts @garethbowen @craig-landry ?

[garethbowen]

I agree we should not have middleware to modify the results from a library which then breaks the API of that library. That would be fragile and very difficult to diagnose when it goes wrong. For this specific case if you're worried about someone adding middleware, or PouchDB breaking their API, we should ensure we have sufficient integration test coverage to prove the components are working together as expected.