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

Custom user session fields can't always be displayed #210

Open
johnbillion opened this issue Sep 15, 2018 · 3 comments
Open

Custom user session fields can't always be displayed #210

johnbillion opened this issue Sep 15, 2018 · 3 comments

Comments

@johnbillion
Copy link
Contributor

When a user session is created, it's possible to add custom fields to the session via the attach_session_information filter. Example:

add_filter( 'attach_session_information', function( array $data, $user_id ) {
    $data['custom'] = 123;
    return $data;
}, 10, 2 );

The wp user session list command in WP-CLI supports a --fields parameter which allows you to include your custom field in the output. Example:

wp user session list john --fields=custom,login_time
+--------+---------------------+
| custom | login_time          |
+--------+---------------------+
| 123    | 2018-09-15 14:28:56 |
+--------+---------------------+

Unfortunately, WP-CLI is a little over-zealous with its field name validation. If any session in the result set does not contain a field called custom, the command fails with the error:

Error: Invalid field: custom.

This situation is easy to experience. Any session that existed before the custom field was implemented will cause the command to fail. Alternatively, if the addition of the custom field is conditional then any session which doesn't get the field attached will also cause the command to fail.

I think the solution is to remove the field name validation, but this may introduce problems with data validation. Suggestions welcome.

@schlessera
Copy link
Member

Shouldn't it be enough to change the validation from "all sessions must contain field" to "at least one session must contain field"? The sessions that don't contain the field can default to just be empty then.

@johnbillion
Copy link
Contributor Author

That will still trigger the error in the situation where none of the sessions contain the field.

The problem is that user sessions don't have the concept of meta data - custom fields are first class properties of the session, but their keys are arbitrary.

I think it would make the most sense to remove the custom field name validation, and simply return an empty value for each session that doesn't include that field. Not ideal though.

@schlessera
Copy link
Member

@johnbillion Maybe then removing the validation, but producing a warning when none of the sessions include that field would be best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants