Skip to content

Updating a Roles User doesn't fire LQ. #7270

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
4 of 6 tasks
dblythy opened this issue Mar 16, 2021 · 3 comments
Closed
4 of 6 tasks

Updating a Roles User doesn't fire LQ. #7270

dblythy opened this issue Mar 16, 2021 · 3 comments

Comments

@dblythy
Copy link
Member

dblythy commented Mar 16, 2021

New Issue Checklist

Possibly related: #5839

Issue Description

If a user is listening to LQ events that they have read access to, and the ACL of an existing Parse.Object (that has the className of the LQ) is updated, an LQ event will fire.

However, this isn't the case with roles.

If a user is listening to LQ events that they have read access to, and the ACL's role of an existing Parse.Object (that has the className of the LQ) add are removes a user, objects that are now available to the user won't come through LQ.

Steps to reproduce

it('expect LQ to work with roles', async done => {
    await reconfigureServer({
      liveQuery: {
        classNames: ['TestObject'],
      },
      startLiveQueryServer: true,
      verbose: false,
      silent: true,
    });

    // setup objects
    const user = new Parse.User();
    user.setUsername('username');
    user.setPassword('password');
    await user.signUp();
    
    const role = new Parse.Role('TestRole', new Parse.ACL(user));
    await role.save();

    const object = new TestObject();
    const acl = new Parse.ACL();
    acl.setRoleReadAccess('TestRole', true);
    object.setACL(acl);
    await object.save(null, {useMasterKey: true});
    
    const query = new Parse.Query(TestObject);
    const subscription = await query.subscribe();

    subscription.on('enter', () => {
      done();
    });
    
    // add user to role
    role.getUsers().add(user);
    await role.save();

    setTimeout(() => {
      fail('LQ should be fired on ACL role update');
    }, 5000);
});

Actual Outcome

No LQ event.

Expected Outcome

When a role user is added or removed, Parse should internally query LQ classes for objects with that role name (possibly limited). If objects are found, they should be passed as LQ events to the client, as an enter or leave request."

This could be problematic and cause bottlenecks with a few LQ classes.

Alternatives:

beforeSave on role and maybe the option to Parse.Cloud.forceLiveQueryFind(Parse.User, 'className'), which could force an LQ to .find all related objects, with a new SDK method:

subscription.on('find', (objects) => {
  for (const obj of objects) {
    if (!allObjects.includes(obj)) {
      allObjects.push(obj);
    }
  }
});

Failing Test Case / Pull Request

  • 🤩 I submitted a PR with a fix and a test case.
  • 🧐 I submitted a PR with a failing test case.

Also related: Community discussion

@mtrezza mtrezza added the type:bug Impaired feature or lacking behavior that is likely assumed label Apr 5, 2021
@percypyan
Copy link
Contributor

Hi ! The LiveQuery is listening for changes on the User collection, however when the you're adding or removing a user to a role, you are updating a role, not a user. This is why the LiveQuery isn't triggered.
On the other hand when you're update ACLs of an object, you are modify the object itself (ACLs being a field of the collection), so the LiveQuery is triggered as expected.

To be confirmed, but I think that is not a bug :)

@mtrezza
Copy link
Member

mtrezza commented Apr 12, 2021

@percypyan Thanks for the comment, after reading the issue again I agree that this seems more like a proposal for an enhancement than a bug.

Or is this even a non-issue, because the LQ trigger should be on the role class, @dblythy?

@mtrezza mtrezza added 🧬 enhancement and removed type:bug Impaired feature or lacking behavior that is likely assumed 🧬 enhancement labels Apr 12, 2021
@dblythy
Copy link
Member Author

dblythy commented Apr 12, 2021

Thank you @percypyan and @mtrezza for the thoughts. I think you both might be right. Let me check and I’ll let you know.

@dblythy dblythy closed this as completed Sep 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants