User Preferences?

[amirfefer]

As part of the Tour component PR, I searched for a persistent mechanism for seen tours

(If a user already observed a tour and clicked on Got it, it shouldn’t be popped out again)

At first, I implemented it as a seen list blob within the user table

But, on secondary thought, why not to extend it to a generic user preferences table?

Foreman already has a user’s table preferences table, which store user’s picked columns, how about generalize it ?

Table preferences attributes

name: string, columns: serialized

Suggested User Preferences Attributes

name: string, kind: string, value: serialized

Data example for records

1. ‘breadcrumbs’, ‘tour’, {alreadySeen: true}
2. ‘hosts’, ‘table’, {columns: {name,...} }
3. ‘per_page’, ‘settings’, 5

[ohadlevy]

I assume this will introduce a new API? (or potentially you would keep the old routes?)

so on top of
GET /api/users/:user_id/table_preferences
you would add?
GET /api/user_preferences

for reference, this was introduced at https://github.com/theforeman/foreman/commit/e73b804e338616ebc536c77dfe8d679fce09cddb

[ekohl]

Makes sense. I’d also add a user level setting to never show tours. If a user maintains multiple foreman instances, they might get annoyed quickly. Other examples are automated testing where you don’t want to see any tours.

[amirfefer]

These routes are not so old so how about introducing a new API ?

[sharvit]

Make sense to me to reuse to current table-preferences, +1 for that.

@ekohl won’t you prefer to keep such a configuration in a seetings.yaml file?

[ohadlevy]

These routes are not so old so how about introducing a new API ?

once published in a release (in this case 1.18) its part of our API… while in theory AFAIK this is only used by katello (and I assume only have an effect on specific tables in katello), but in the past we never changed a public API endpoint, this case might be different…

[ohadlevy]

@ekohl won’t you prefer to keep such a configuration in a seetings.yaml file?

I would prefer the settings table (vs the file) as it does not impact the server ability to load (or in other words, you dont have to restart rails if this option changes).

[ekohl]

As @ohadlevy said: a global setting doesn’t cover all cases.

In a production setup the admin might have upgraded a test instance and verified all functionality so knows the setups. They’ll disable tours on their production user. The same org has other users that might want to follow tours.

In automated testing you might want to verify tours, so you create a new user and let them follow tours. Another scenario is just testing the functionality so you create a user with tours disabled. With per user settings you can do this in parallel and speed up the test suite.

That said, a global setting could work, but that can also be implemented as the default when creating users.

[ohadlevy]

In a production setup the admin might have upgraded a test instance and verified all functionality so knows the setups. They’ll disable tours on their production user. The same org has other users that might want to follow tours.

Are you also expecting a per org setting? I would prefer to avoid it initially if we can…?

[ekohl]

I don’t see a need for per org settings that per user doesn’t cover. I agree we’ll probably want to avoid it because it brings a lot of complexity for IMHO minimal value.

[ohadlevy]

I don’t see a need for per org settings that per user doesn’t cover. I agree we’ll probably want to avoid it because it brings a lot of complexity for IMHO minimal value.

the main feature would be that you can set it once for current and future users in your org. I agree this is out of scope.

[Tomas_Strachota]

Migrating from table_preferences to user_preferences makes sense to me too. So far it’s only Katello that uses this API so it shouldn’t be a big deal. It would be good if we can change the models, add the API and keep the legacy routes & params for one release.

The ability to disable tours per-user as @ekohl suggests seems reasonable.

[Walden]

This is currently only used in the selectable columns implementation on the subscriptions page in katello. I think that this may be an exception where we could change the API in place without a new version and without deprecating the old endpoint. The changes in katello would be minimal I think.

@Partha_Aji @Partha_Aji (not sure which name to use) what are your thoughts on changing this API as discussed above?

[Partha_Aji]

I worked on this purely s to help the UI team. I don’t think any one outside is using it. As long as we can guarantee porting subscriptions page in katello to work with the new setup I would have no objections.

[ohadlevy]

I worked on this purely s to help the UI team. I don’t think any one outside is using it. As long as we can guarantee porting subscriptions page in katello to work with the new setup I would have no objections.

@amirfefer I would suggest to move to a PR ?

[amirfefer]

I’ve updated the tour PR already