Skip to content
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

[Feature] Be able to define default user fields published on login #11118

Conversation

menelike
Copy link
Contributor

@menelike menelike commented Jul 10, 2020

This removes the limitation described in https://docs.meteor.com/api/accounts.html

By default, the current user’s username, emails and profile are published to the client. You can publish additional fields for the current user with:

// Server
Meteor.publish('userData', function () {
  if (this.userId) {
    return Meteor.users.find({ _id: this.userId }, {
      fields: { other: 1, things: 1 }
    });
  } else {
    this.ready();
  }
});

// Client
Meteor.subscribe('userData');
How to be used
Accounts.setDefaultPublishFields({username: 1, profile: 1, emails: 1, 'foo.bar': 1});
// or
Accounts.setDefaultPublishFields({'do.not.publish.this': -1});

Since this replaces the default projection the user has to add profile, username, and profile to the projection to keep the old behavior.

Motivation

I think that writing an additional subscription just for the sole purpose to fetch additional fields, is cumbersome and degrades the performance for no reason.

Background

In our scenario, a logged-in user has additional fields that are not part of profile. Those fields are needed to determine the state and the permissions of the user. In the UI, after the login has succeeded, the user will be redirected to the primary route for logged-in users - this then checks for the appropriate permission. At that point, the custom subscription used to fetch all the user data has not finished yet. As a result, the user will be redirected back to the "you are not logged in" page.

tl;dr
It helps with race conditions where fields are missing after login.

…ds published on user login

- this is implemented in the same way of the Autopublish package in conjunction with Accounts.addAutopublishFields()
Copy link
Collaborator

@filipenevola filipenevola left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice addition, thanks. I'm requesting a change in the format 😉

packages/accounts-base/accounts_server.js Outdated Show resolved Hide resolved
packages/accounts-base/accounts_server.js Outdated Show resolved Hide resolved
@menelike
Copy link
Contributor Author

@filipenevola Hope it fits now. I think that replacing the default projection makes more sense than extending it (which would have to deal with mixed projections). WDYT?

@menelike menelike force-pushed the extendDefaultUserPublishedFields branch from 4b3d2bf to ee2f65d Compare July 30, 2020 09:03
@filipenevola
Copy link
Collaborator

@menelike I don't know why CircleCI is trying to run the tests in your account and not at Meteor org. Could you create a new branch and a new PR? Sometimes CircleCI get lost :(

@afrokick
Copy link
Contributor

@menelike

…faultPublishFields(object)

Instead of extending the default projection it is overridden. Otherwise a check for mixed projections would have been necessary. In addition, replacing the default projection gives the user full control over the result.
@menelike menelike force-pushed the extendDefaultUserPublishedFields branch from ee2f65d to 78ea663 Compare August 19, 2020 07:34
@menelike
Copy link
Contributor Author

@afrokick @filipenevola

I was able to work around the issues with circleCI (bugs on their side), all tests have passed, we should be good to go.

@filipenevola filipenevola added the confirmed We want to fix or implement it label Nov 5, 2020
@filipenevola filipenevola added this to the Release 2.0 milestone Nov 5, 2020
@filipenevola filipenevola changed the base branch from devel to release-2.0 November 7, 2020 15:41
@filipenevola filipenevola merged commit f93183e into meteor:release-2.0 Nov 7, 2020
@filipenevola
Copy link
Collaborator

Cherry-picked to 1.12

@santiagopuentep
Copy link

santiagopuentep commented Dec 8, 2020

@filipenevola I'm a little confused. What's the difference between Accounts.setDefaultPublishFields and Accounts.config.defaultFieldsSelector? They seems to do the same thing.

@filipenevola
Copy link
Collaborator

Hi @santiagopuentep, it's used specific for the publication and not for the finds as a whole.

See the diff below, in the previous code Meteor was always publishing 3 specific fields:
accounts_server js (:Users:filipe:Documents:meteor:ws:meteor:packages:accounts-base) 2020-12-10 17-13-17

@mullojo
Copy link
Contributor

mullojo commented Jun 14, 2021

Hi @menelike

Is this the correct syntax for how to use this update?

Accounts.setDefaultPublishFields({username: 1, profile: 1, emails: 1, 'foo.bar': 1});
// or
Accounts.setDefaultPublishFields({'do.not.publish.this': -1});

It would be good to put these updates in the accounts docs: https://docs.meteor.com/api/accounts.html

Also, I'm uncertain if the defaults for the accounts-base package are still the same or if the user needs to specify them as this thread implies. It would be really great if there was just a bit of clarification added and it would be great to see updates to the Meteor docs.

Would this update change the behavior of calling data from Meteor.user() on the client by default? I found this thread on the Meteor Forums where a user had an issue with Meteor.user() after upgrading to v1.12: https://forums.meteor.com/t/meteor-user-returns-undefined-after-updating-to-1-12/54843/16

@filipenevola if you know how these updates work, it would be great if you could share them too. In the Meteor v1.12 update, this is not specified as a breaking change, but it would be if the previous defaults have changes.

@StorytellerCZ StorytellerCZ added missing-docs Documentation or history entry is missing for the PR and removed confirmed We want to fix or implement it labels Jun 15, 2021
@menelike
Copy link
Contributor Author

Hi @mullojo

Is this the correct syntax for how to use this update?

Yes.

It would be good to put these updates in the accounts docs: https://docs.meteor.com/api/accounts.html

Absolutely!

Would this update change the behavior of calling data from Meteor.user() on the client by default? I found this thread on the Meteor Forums where a user had an issue with Meteor.user() after upgrading to v1.12: https://forums.meteor.com/t/meteor-user-returns-undefined-after-updating-to-1-12/54843/16

I've written a comment in the linked thread. Generally speaking Accounts.setDefaultPublishFields(); is an optional call, if not used the default fields are published which makes this backward compatible. I don't see any reasons for a breaking change. But keep in mind that if you use Accounts.setDefaultPublishFields(); you need to pass all fields you want to be published to the client as the default fields are then omitted.

@menelike menelike deleted the extendDefaultUserPublishedFields branch June 15, 2021 07:04
@rj-david
Copy link
Contributor

We had a lot of handling added in our app to manage that race condition that the other fields are not yet available for the UI to check. Thanks for this, @menelike.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
missing-docs Documentation or history entry is missing for the PR Project:Accounts (in user apps) Type:Feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants