[BUG] /api/v1/me returns only preferences that are not default

[cardoso]

It doesn't make sense for us to hardcode the default settings in our client.

We should receive all preferences, not only the ones that changed.

Issue in the docs for reference:
https://github.com/RocketChat/docs/issues/695

[sampaiodiego]

I know it can become a mess having default values hard coded on each client, but not returning them from APIs also helps saving client's bandwidth.

what @RocketChat/core thinks about this? should we have the logic spread on each client or should we make clients use a bit more data?

[geekgonecrazy]

Had to sit on this one and think a while.... But I have an opinion 😄

I think what ever the current value is, default or otherwise... we probably should send. For the sake of the clients especially in case of: @rocketchat/ios @rocketchat/android

I think the amount of bytes saved here aren't going to be worth the extra work of having to implement in every client. If When we implement new preferences they will then have to go and add this new settings value hard coded. If we change the settings default... they all will have to update the clients hard coded value.

This is a topic for another time. But I think we should combine all settings needed for any client into a single endpoint to decrease the number of calls needed for any client(even the web client) to get up and running.

Instead of having to make multiple subscriptions and wait on preferences, and settings both to be delivered over a websocket.

But that's just an extra 2 cents.

[graywolf336]

I'm conflicted on this one. I feel like the user's preferences should be passed alongside the default ones to each client, but what happens when the server changes the type of the setting/preference? And how will the clients handle the possibility of different types of the values? Or do we need to make a very pointed effort to not change types of preferences anymore and instead deprecate ones and add new ones to replace the deprecated one?

[rafaelks]

I can see your points @sampaiodiego, @geekgonecrazy, @graywolf336... IMHO, we don't need to save bandwidth on this situation, because all clients need this data in order to work correctly. This argument doesn't make any sense when we know that if the setting changed, the data is actually returned, and no bandwidth is saved. In order to save bandwidth, I believe need to put effort on fields that no client care about.

I see two main advantages in having the default values being returned on the APIs:

1. It's easy to understand the possibilities. As a new client or a new developer using our APIs, they'll know most of the possibilities just by looking into the first API response they have (we don't have all those fields in our docs today, but that's a different problem).
2. We trust one source for the default behaviour. All clients will behaviour the same way on the initial setup.

Regarding your comment about changing values @graywolf336: I think it's not a problem to add a new value to a preference and setting it as the default one. The more we can communicate it, the better. But the clients can "protect" itself from getting invalid values and use some default that makes sense in case they receive a value not expected.

[rafaelks]

@RocketChat/core Any updates on this one guys?

[luciofm]

And how will the clients handle the possibility of different types of the values?

We already have hard time on mobile with attributes that are not always the same type like when there is just one value it is an object and when there are more it is an array of objects, fields that you are expecting a boolean and get a String.

on Android, at least, ideally I'd have a model like this

@JsonSerializable
data class(val id: Long, val name: String, val file_size: Int, val use_realname: Boolean, val list: List<Int>)

and that's it, this will auto generate an effiecient mapper from/to Json to Java/Kotlin object.
But any time we find an attribute that has multiple types, we need to write a mapper by hand.

So please, anytime when changing the type, create a new value and deprecate the old one. Not only for settings.

About saving bandwidth we could have a query just like in public settings, requesting only the preferences that the client care about...

Just for example, the new Android App asks just this settings

    private var settingsFilter = arrayOf(
        LDAP_ENABLE, CAS_ENABLE, CAS_LOGIN_URL,

        ACCOUNT_REGISTRATION, ACCOUNT_LOGIN_FORM, ACCOUNT_PASSWORD_RESET, ACCOUNT_CUSTOM_FIELDS,
        ACCOUNT_GOOGLE, ACCOUNT_FACEBOOK, ACCOUNT_GITHUB, ACCOUNT_LINKEDIN, ACCOUNT_METEOR,
        ACCOUNT_TWITTER, ACCOUNT_WORDPRESS, ACCOUNT_GITLAB,

        SITE_URL, SITE_NAME, FAVICON_512, FAVICON_196, USE_REALNAME, ALLOW_ROOM_NAME_SPECIAL_CHARS,
        FAVORITE_ROOMS, UPLOAD_STORAGE_TYPE, UPLOAD_MAX_FILE_SIZE, UPLOAD_WHITELIST_MIMETYPES,
        HIDE_USER_JOIN, HIDE_USER_LEAVE,
        HIDE_TYPE_AU, HIDE_MUTE_UNMUTE, HIDE_TYPE_RU, ALLOW_MESSAGE_DELETING,
        ALLOW_MESSAGE_EDITING, ALLOW_MESSAGE_PINNING, SHOW_DELETED_STATUS, SHOW_EDITED_STATUS,
        WIDE_TILE_310)

[engelgabriel]

@MarcosSpessatto we need to make sure all user preferences have a default in the admin settings, so there is nothing hardcoded on the UI. Should @karlprieb and @ggazzo be involved here?

[sampaiodiego]

@engelgabriel recently we decided to drop the settings for Desktop Notifications Default Alert and Mobile Notifications Default Alert because there are others server wide settings for the same purpose.

[sampaiodiego]

oh darn 🤦‍♂️ I had a local instance where a migration did not run so I had two settings for Desktop Notifications Default Alert and Mobile Notifications Default Alert .. but on the current state, we have only one, and it is on Default User Preferences section.. so I think we're good 👍