Help wanted testing PR

packages
1

I had a couple of requests for improved functionality within tunguska:reactive-aggregate and I put a PR together which addresses those and adds some additional functionality.

Unfortunately, despite asking the original issue owners to test the PR, I have had no success.

Given the nature of the changes I’ve made (which should be backwards-compatible), I’m reluctant to Just Merge It™.

If anyone currently using tunguska:reactive-aggregate would like to help test this PR, either for backwards compatibility (i.e. does it still work without changing your code), or to test the new features, it would be really helpful.

Thanks :slight_smile:

6 Likes
2

I’ll try to give it a spin, but I noticed the API was changed. Below is what we had before in one of the publications, we’ll need to convert to the new API with the observers option to test it.

Until I take a closer look at the code, any suggestion on how re-write the query below would speed the testing.

Thanks!

  ReactiveAggregate(
    this,
    Comments,
    [
      { $match: { postId } },
      {
        $lookup: {
          from: 'users',
          let: { userId: '$userId' },
          pipeline: [
            {
              $match: {
                $expr: {
                  $eq: ['$_id', '$$userId']
                }
              }
            },
            {
              $project: {
                _id: 1,
                emails: 1,
                profile: 1
              }
            }
          ],
          as: 'user'
        }
      },
      {
        $project: projectedField
      }
    ],
    {
      observeOptions: {
        fields: Comments.publicFields,
        sort: {
          createdAt: 1
        }
      },
      clientCollection: 'comments'
    }
  );
});
3 Likes
3

The API should be backwards compatible, so no need for any changes unless you want to take advantage of the new features.

1 Like
4

Alright, so I got a chance to do a bit more testing. In order to restore the old behaviour, I had to make few changes to the submitted PR.

  1. In the default localOptions, I changed noAutomaticObserver to false to add the old cursor by default
  2. I changed the default debounce count to 0 to prevent skipping updates since the old behaviour didn’t skip
  3. I changed if (currentDebounceCount++ > localOptions.debounceCount) to (++currentDebounceCount > localOptions.debounceCount) which triggered the update when the debounceCount is zero, without that, it was only firing every second update when the debounceCount is set to zero.

With those 3 changes, the query I’ve shared above worked as before. I hope that helps with the testing!

3 Likes
5

Sorry for the late reply, Family commitments and illness has kept me away from the forums.

Many thanks for finding the time to run through the code and come up with the changes you’ve suggested. I’ll try and implement those later today - unless you’d like to put a PR together on top of mine?

1 Like