diff options
-rw-r--r-- | api/preference.go | 22 | ||||
-rw-r--r-- | api/preference_test.go | 10 | ||||
-rw-r--r-- | model/preference.go | 15 | ||||
-rw-r--r-- | model/preference_test.go | 11 | ||||
-rw-r--r-- | store/sql_preference_store.go | 8 | ||||
-rw-r--r-- | store/sql_preference_store_test.go | 18 | ||||
-rw-r--r-- | web/react/components/sidebar.jsx | 4 | ||||
-rw-r--r-- | web/react/stores/preference_store.jsx | 22 |
8 files changed, 56 insertions, 54 deletions
diff --git a/api/preference.go b/api/preference.go index 2f5ba12dd..88cb132f8 100644 --- a/api/preference.go +++ b/api/preference.go @@ -14,9 +14,9 @@ func InitPreference(r *mux.Router) { l4g.Debug("Initializing preference api routes") sr := r.PathPrefix("/preferences").Subrouter() - sr.Handle("/save", ApiAppHandler(savePreferences)).Methods("POST") - sr.Handle("/{category:[A-Za-z0-9_]+}", ApiAppHandler(getPreferenceCategory)).Methods("GET") - sr.Handle("/{category:[A-Za-z0-9_]+}/{name:[A-Za-z0-9_]+}", ApiAppHandler(getPreference)).Methods("GET") + sr.Handle("/save", ApiUserRequired(savePreferences)).Methods("POST") + sr.Handle("/{category:[A-Za-z0-9_]+}", ApiUserRequired(getPreferenceCategory)).Methods("GET") + sr.Handle("/{category:[A-Za-z0-9_]+}/{name:[A-Za-z0-9_]+}", ApiUserRequired(getPreference)).Methods("GET") } func savePreferences(c *Context, w http.ResponseWriter, r *http.Request) { @@ -52,17 +52,21 @@ func getPreferenceCategory(c *Context, w http.ResponseWriter, r *http.Request) { } else { data := result.Data.(model.Preferences) - if len(data) == 0 { - if category == model.PREFERENCE_CATEGORY_DIRECT_CHANNEL_SHOW { - // add direct channels for a user that existed before preferences were added - data = addDirectChannels(c.Session.UserId, c.Session.TeamId) - } - } + data = transformPreferences(c, data, category) w.Write([]byte(data.ToJson())) } } +func transformPreferences(c *Context, preferences model.Preferences, category string) model.Preferences { + if len(preferences) == 0 && category == model.PREFERENCE_CATEGORY_DIRECT_CHANNEL_SHOW { + // add direct channels for a user that existed before preferences were added + preferences = addDirectChannels(c.Session.UserId, c.Session.TeamId) + } + + return preferences +} + func addDirectChannels(userId, teamId string) model.Preferences { var profiles map[string]*model.User if result := <-Srv.Store.User().GetProfiles(teamId); result.Err != nil { diff --git a/api/preference_test.go b/api/preference_test.go index d9fdc2877..318ce9582 100644 --- a/api/preference_test.go +++ b/api/preference_test.go @@ -71,13 +71,13 @@ func TestGetPreferenceCategory(t *testing.T) { user2 = Client.Must(Client.CreateUser(user2, "")).Data.(*model.User) store.Must(Srv.Store.User().VerifyEmail(user2.Id)) - category := model.PREFERENCE_CATEGORY_TEST + category := model.NewId() preferences1 := model.Preferences{ { UserId: user1.Id, Category: category, - Name: model.PREFERENCE_NAME_TEST, + Name: model.NewId(), }, { UserId: user1.Id, @@ -86,8 +86,8 @@ func TestGetPreferenceCategory(t *testing.T) { }, { UserId: user1.Id, - Category: model.PREFERENCE_CATEGORY_TEST, - Name: model.PREFERENCE_NAME_TEST, + Category: model.NewId(), + Name: model.NewId(), }, } @@ -113,7 +113,7 @@ func TestGetPreferenceCategory(t *testing.T) { } } -func TestGetDefaultDirectChannels(t *testing.T) { +func TestTransformPreferences(t *testing.T) { Setup() team := &model.Team{DisplayName: "Name", Name: "z-z-" + model.NewId() + "a", Email: "test@nowhere.com", Type: model.TEAM_OPEN} diff --git a/model/preference.go b/model/preference.go index 187dbe848..44279f71a 100644 --- a/model/preference.go +++ b/model/preference.go @@ -10,8 +10,6 @@ import ( const ( PREFERENCE_CATEGORY_DIRECT_CHANNEL_SHOW = "direct_channel_show" - PREFERENCE_CATEGORY_TEST = "test" // do not use, just for testing uniqueness while there's only one real category - PREFERENCE_NAME_TEST = "test" // do not use, just for testing while there's no constant name ) type Preference struct { @@ -46,12 +44,11 @@ func (o *Preference) IsValid() *AppError { return NewAppError("Preference.IsValid", "Invalid user id", "user_id="+o.UserId) } - if len(o.Category) == 0 || len(o.Category) > 32 || !IsPreferenceCategoryValid(o.Category) { + if len(o.Category) == 0 || len(o.Category) > 32 { return NewAppError("Preference.IsValid", "Invalid category", "category="+o.Category) } - // name can either be a valid constant or an id - if len(o.Name) == 0 || len(o.Name) > 32 || !(len(o.Name) == 26 || IsPreferenceNameValid(o.Name)) { + if len(o.Name) == 0 || len(o.Name) > 32 { return NewAppError("Preference.IsValid", "Invalid name", "name="+o.Name) } @@ -61,11 +58,3 @@ func (o *Preference) IsValid() *AppError { return nil } - -func IsPreferenceCategoryValid(category string) bool { - return category == PREFERENCE_CATEGORY_DIRECT_CHANNEL_SHOW || category == PREFERENCE_CATEGORY_TEST -} - -func IsPreferenceNameValid(name string) bool { - return name == PREFERENCE_NAME_TEST -} diff --git a/model/preference_test.go b/model/preference_test.go index 170784c1f..66b7ac50b 100644 --- a/model/preference_test.go +++ b/model/preference_test.go @@ -12,7 +12,7 @@ func TestPreferenceIsValid(t *testing.T) { preference := Preference{ UserId: "1234garbage", Category: PREFERENCE_CATEGORY_DIRECT_CHANNEL_SHOW, - Name: PREFERENCE_NAME_TEST, + Name: NewId(), } if err := preference.IsValid(); err == nil { @@ -24,7 +24,7 @@ func TestPreferenceIsValid(t *testing.T) { t.Fatal(err) } - preference.Category = "1234garbage" + preference.Category = strings.Repeat("01234567890", 20) if err := preference.IsValid(); err == nil { t.Fatal() } @@ -34,7 +34,7 @@ func TestPreferenceIsValid(t *testing.T) { t.Fatal() } - preference.Name = "1234garbage" + preference.Name = strings.Repeat("01234567890", 20) if err := preference.IsValid(); err == nil { t.Fatal() } @@ -44,11 +44,6 @@ func TestPreferenceIsValid(t *testing.T) { t.Fatal() } - preference.Name = PREFERENCE_NAME_TEST - if err := preference.IsValid(); err != nil { - t.Fatal() - } - preference.Value = strings.Repeat("01234567890", 20) if err := preference.IsValid(); err == nil { t.Fatal() diff --git a/store/sql_preference_store.go b/store/sql_preference_store.go index 59c52d888..46cef38b1 100644 --- a/store/sql_preference_store.go +++ b/store/sql_preference_store.go @@ -162,9 +162,9 @@ func (s SqlPreferenceStore) Get(userId string, category string, name string) Sto go func() { result := StoreResult{} - var preferences model.Preferences + var preference model.Preference - if _, err := s.GetReplica().Select(&preferences, + if err := s.GetReplica().SelectOne(&preference, `SELECT * FROM @@ -173,9 +173,9 @@ func (s SqlPreferenceStore) Get(userId string, category string, name string) Sto UserId = :UserId AND Category = :Category AND Name = :Name`, map[string]interface{}{"UserId": userId, "Category": category, "Name": name}); err != nil { - result.Err = model.NewAppError("SqlPreferenceStore.GetByName", "We encounted an error while finding preferences", err.Error()) + result.Err = model.NewAppError("SqlPreferenceStore.Get", "We encounted an error while finding preferences", err.Error()) } else { - result.Data = preferences[0] + result.Data = preference } storeChannel <- result diff --git a/store/sql_preference_store_test.go b/store/sql_preference_store_test.go index 1feda01d9..76b1bcb17 100644 --- a/store/sql_preference_store_test.go +++ b/store/sql_preference_store_test.go @@ -55,7 +55,7 @@ func TestPreferenceGet(t *testing.T) { userId := model.NewId() category := model.PREFERENCE_CATEGORY_DIRECT_CHANNEL_SHOW - name := model.PREFERENCE_NAME_TEST + name := model.NewId() preferences := model.Preferences{ { @@ -70,7 +70,7 @@ func TestPreferenceGet(t *testing.T) { }, { UserId: userId, - Category: model.PREFERENCE_CATEGORY_TEST, + Category: model.NewId(), Name: name, }, { @@ -87,6 +87,11 @@ func TestPreferenceGet(t *testing.T) { } else if data := result.Data.(model.Preference); data != preferences[0] { t.Fatal("got incorrect preference") } + + // make sure getting a missing preference fails + if result := <-store.Preference().Get(model.NewId(), model.NewId(), model.NewId()); result.Err == nil { + t.Fatal("no error on getting a missing preference") + } } func TestPreferenceGetCategory(t *testing.T) { @@ -111,7 +116,7 @@ func TestPreferenceGetCategory(t *testing.T) { // same user/name, different category { UserId: userId, - Category: model.PREFERENCE_CATEGORY_TEST, + Category: model.NewId(), Name: name, }, // same name/category, different user @@ -131,4 +136,11 @@ func TestPreferenceGetCategory(t *testing.T) { } else if !((data[0] == preferences[0] && data[1] == preferences[1]) || (data[0] == preferences[1] && data[1] == preferences[0])) { t.Fatal("got incorrect preferences") } + + // make sure getting a missing preference category doesn't fail + if result := <-store.Preference().GetCategory(model.NewId(), model.NewId()); result.Err != nil { + t.Fatal(result.Err) + } else if data := result.Data.(model.Preferences); len(data) != 0 { + t.Fatal("shouldn't have got any preferences") + } } diff --git a/web/react/components/sidebar.jsx b/web/react/components/sidebar.jsx index 82fc828eb..1caf4caa5 100644 --- a/web/react/components/sidebar.jsx +++ b/web/react/components/sidebar.jsx @@ -310,7 +310,9 @@ export default class Sidebar extends React.Component { this.isLeaving.set(channel.id, true); const preference = PreferenceStore.setPreference(Constants.Preferences.CATEGORY_DIRECT_CHANNEL_SHOW, channel.teammate_id, 'false'); - AsyncClient.savePreferences( + + // bypass AsyncClient since we've already saved the updated preferences + Client.savePreferences( [preference], () => { this.isLeaving.set(channel.id, false); diff --git a/web/react/stores/preference_store.jsx b/web/react/stores/preference_store.jsx index 4d7fb9ad6..d71efa10f 100644 --- a/web/react/stores/preference_store.jsx +++ b/web/react/stores/preference_store.jsx @@ -9,6 +9,14 @@ const UserStore = require('../stores/user_store.jsx'); const CHANGE_EVENT = 'change'; +function getPreferenceKey(category, name) { + return `${category}-${name}`; +} + +function getPreferenceKeyForModel(preference) { + return `${preference.category}-${preference.name}`; +} + class PreferenceStoreClass extends EventEmitter { constructor() { super(); @@ -28,20 +36,12 @@ class PreferenceStoreClass extends EventEmitter { this.dispatchToken = AppDispatcher.register(this.handleEventPayload); } - getKey(category, name) { - return `${category}-${name}`; - } - - getKeyForModel(preference) { - return `${preference.category}-${preference.name}`; - } - getAllPreferences() { return new Map(BrowserStore.getItem('preferences', [])); } getPreference(category, name, defaultValue = '') { - return this.getAllPreferences().get(this.getKey(category, name)) || defaultValue; + return this.getAllPreferences().get(getPreferenceKey(category, name)) || defaultValue; } getPreferences(category) { @@ -70,7 +70,7 @@ class PreferenceStoreClass extends EventEmitter { setPreference(category, name, value) { const preferences = this.getAllPreferences(); - const key = this.getKey(category, name); + const key = getPreferenceKey(category, name); let preference = preferences.get(key); if (!preference) { @@ -109,7 +109,7 @@ class PreferenceStoreClass extends EventEmitter { const preferences = this.getAllPreferences(); for (const preference of action.preferences) { - preferences.set(this.getKeyForModel(preference), preference); + preferences.set(getPreferenceKeyForModel(preference), preference); } this.setAllPreferences(preferences); |