From be9feeb9187adf84882d5553c9aba3e84b83f87f Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Thu, 26 Feb 2015 12:25:25 -0500 Subject: [PATCH] Replace `CurrentUserMixin` with an injected `currentUser` This is a lot simpler and removes the need for stubbing singletons in unit tests. --- .../discourse/controllers/controller.js.es6 | 2 +- .../controllers/notifications.js.es6 | 2 +- .../discourse/controllers/object.js.es6 | 2 +- .../controllers/site-map-category.js.es6 | 2 +- .../discourse/controllers/site-map.js.es6 | 2 +- .../controllers/user-dropdown.js.es6 | 4 ++-- .../controllers/user-topics-list.js.es6 | 2 +- .../initializers/inject-objects.js.es6 | 2 +- .../discourse/mixins/has_current_user.js | 12 ------------ .../routes/build-category-route.js.es6 | 3 +-- .../templates/navigation/category.hbs | 2 +- .../controllers/discourse-test.js.es6 | 1 - .../controllers/header-test.js.es6 | 19 ++++++++----------- .../controllers/notifications-test.js.es6 | 7 ------- .../controllers/site-map-test.js.es6 | 17 ++++++----------- .../controllers/user-dropdown-test.js.es6 | 18 +++++++----------- .../mixins/has-current-user-test.js.es6 | 12 ------------ 17 files changed, 32 insertions(+), 77 deletions(-) delete mode 100644 app/assets/javascripts/discourse/mixins/has_current_user.js delete mode 100644 test/javascripts/controllers/notifications-test.js.es6 delete mode 100644 test/javascripts/mixins/has-current-user-test.js.es6 diff --git a/app/assets/javascripts/discourse/controllers/controller.js.es6 b/app/assets/javascripts/discourse/controllers/controller.js.es6 index 91fabd74710..94c3ba1d43e 100644 --- a/app/assets/javascripts/discourse/controllers/controller.js.es6 +++ b/app/assets/javascripts/discourse/controllers/controller.js.es6 @@ -1 +1 @@ -export default Ember.Controller.extend(Discourse.Presence, Discourse.HasCurrentUser); +export default Ember.Controller.extend(Discourse.Presence); diff --git a/app/assets/javascripts/discourse/controllers/notifications.js.es6 b/app/assets/javascripts/discourse/controllers/notifications.js.es6 index 300d22b01a4..1b2c71574c9 100644 --- a/app/assets/javascripts/discourse/controllers/notifications.js.es6 +++ b/app/assets/javascripts/discourse/controllers/notifications.js.es6 @@ -1,4 +1,4 @@ -export default Ember.ArrayController.extend(Discourse.HasCurrentUser, { +export default Ember.ArrayController.extend({ needs: ['header'], loadingNotifications: Em.computed.alias('controllers.header.loadingNotifications') }); diff --git a/app/assets/javascripts/discourse/controllers/object.js.es6 b/app/assets/javascripts/discourse/controllers/object.js.es6 index 56bef4c5199..75a47142857 100644 --- a/app/assets/javascripts/discourse/controllers/object.js.es6 +++ b/app/assets/javascripts/discourse/controllers/object.js.es6 @@ -1 +1 @@ -export default Ember.ObjectController.extend(Discourse.Presence, Discourse.HasCurrentUser); +export default Ember.ObjectController.extend(Discourse.Presence); diff --git a/app/assets/javascripts/discourse/controllers/site-map-category.js.es6 b/app/assets/javascripts/discourse/controllers/site-map-category.js.es6 index cf10e9d6bb8..52da588ccfd 100644 --- a/app/assets/javascripts/discourse/controllers/site-map-category.js.es6 +++ b/app/assets/javascripts/discourse/controllers/site-map-category.js.es6 @@ -1,4 +1,4 @@ -export default Ember.ObjectController.extend(Discourse.HasCurrentUser, { +export default Ember.ObjectController.extend({ needs: ['site-map'], unreadTotal: function() { diff --git a/app/assets/javascripts/discourse/controllers/site-map.js.es6 b/app/assets/javascripts/discourse/controllers/site-map.js.es6 index 8503d90781a..9ee464c789c 100644 --- a/app/assets/javascripts/discourse/controllers/site-map.js.es6 +++ b/app/assets/javascripts/discourse/controllers/site-map.js.es6 @@ -1,4 +1,4 @@ -export default Ember.ArrayController.extend(Discourse.HasCurrentUser, { +export default Ember.ArrayController.extend({ needs: ['application'], showBadgesLink: function(){return Discourse.SiteSettings.enable_badges;}.property(), diff --git a/app/assets/javascripts/discourse/controllers/user-dropdown.js.es6 b/app/assets/javascripts/discourse/controllers/user-dropdown.js.es6 index 3413e61f276..af91f6c6f16 100644 --- a/app/assets/javascripts/discourse/controllers/user-dropdown.js.es6 +++ b/app/assets/javascripts/discourse/controllers/user-dropdown.js.es6 @@ -1,8 +1,8 @@ -export default Ember.ArrayController.extend(Discourse.HasCurrentUser, { +export default Ember.ArrayController.extend({ showAdminLinks: Em.computed.alias("currentUser.staff"), actions: { - logout: function() { + logout() { Discourse.logout(); return false; } diff --git a/app/assets/javascripts/discourse/controllers/user-topics-list.js.es6 b/app/assets/javascripts/discourse/controllers/user-topics-list.js.es6 index 2d252ac1560..5bab37f8386 100644 --- a/app/assets/javascripts/discourse/controllers/user-topics-list.js.es6 +++ b/app/assets/javascripts/discourse/controllers/user-topics-list.js.es6 @@ -1,7 +1,7 @@ import ObjectController from 'discourse/controllers/object'; // Lists of topics on a user's page. -export default ObjectController.extend(Discourse.HasCurrentUser, { +export default ObjectController.extend({ needs: ["application", "user"], hideCategory: false, showParticipants: false, diff --git a/app/assets/javascripts/discourse/initializers/inject-objects.js.es6 b/app/assets/javascripts/discourse/initializers/inject-objects.js.es6 index fc37aad17eb..ba0be1f986c 100644 --- a/app/assets/javascripts/discourse/initializers/inject-objects.js.es6 +++ b/app/assets/javascripts/discourse/initializers/inject-objects.js.es6 @@ -42,9 +42,9 @@ export default { app.inject('view', 'session', 'session:main'); app.inject('model', 'session', 'session:main'); - // Inject currentUser. Components only for now to prevent any breakage app.register('current-user:main', Discourse.User.current(), { instantiate: false }); app.inject('component', 'currentUser', 'current-user:main'); + app.inject('controller', 'currentUser', 'current-user:main'); app.register('store:main', Store); app.inject('route', 'store', 'store:main'); diff --git a/app/assets/javascripts/discourse/mixins/has_current_user.js b/app/assets/javascripts/discourse/mixins/has_current_user.js deleted file mode 100644 index 92d5461a1f4..00000000000 --- a/app/assets/javascripts/discourse/mixins/has_current_user.js +++ /dev/null @@ -1,12 +0,0 @@ -/** - This mixin provides a `currentUser` property that can be used to retrieve information - about the currently logged in user. It is mostly useful to controllers so it can be - exposted to templates. -**/ -Discourse.HasCurrentUser = Em.Mixin.create({ - - currentUser: function() { - return Discourse.User.current(); - }.property().volatile() - -}); diff --git a/app/assets/javascripts/discourse/routes/build-category-route.js.es6 b/app/assets/javascripts/discourse/routes/build-category-route.js.es6 index ae70acd7d6d..e3567ef8506 100644 --- a/app/assets/javascripts/discourse/routes/build-category-route.js.es6 +++ b/app/assets/javascripts/discourse/routes/build-category-route.js.es6 @@ -28,8 +28,7 @@ export default function(filter, params) { category: model, filterMode: filterMode, noSubcategories: params && params.no_subcategories, - canEditCategory: model.get('can_edit'), - canChangeCategoryNotificationLevel: Discourse.User.current() + canEditCategory: model.get('can_edit') }); }, diff --git a/app/assets/javascripts/discourse/templates/navigation/category.hbs b/app/assets/javascripts/discourse/templates/navigation/category.hbs index a7272acf3f2..b53d7ab0b5b 100644 --- a/app/assets/javascripts/discourse/templates/navigation/category.hbs +++ b/app/assets/javascripts/discourse/templates/navigation/category.hbs @@ -10,7 +10,7 @@ {{custom-html "extraNavItem"}} -{{#if canChangeCategoryNotificationLevel}} +{{#if currentUser}} {{category-notifications-button category=category}} {{/if}} diff --git a/test/javascripts/controllers/discourse-test.js.es6 b/test/javascripts/controllers/discourse-test.js.es6 index 03bdd42ed3a..cf5959f8ad3 100644 --- a/test/javascripts/controllers/discourse-test.js.es6 +++ b/test/javascripts/controllers/discourse-test.js.es6 @@ -4,5 +4,4 @@ module("DiscourseController"); test("includes mixins", function() { ok(Discourse.Presence.detect(DiscourseController.create()), "Discourse.Presence"); - ok(Discourse.HasCurrentUser.detect(DiscourseController.create()), "Discourse.HasCurrentUser"); }); diff --git a/test/javascripts/controllers/header-test.js.es6 b/test/javascripts/controllers/header-test.js.es6 index d5bdbe501f5..4b82269b7f6 100644 --- a/test/javascripts/controllers/header-test.js.es6 +++ b/test/javascripts/controllers/header-test.js.es6 @@ -3,26 +3,23 @@ moduleFor("controller:header", "controller:header", { }); test("showNotifications action", function() { - var resolveRequestWith; - var request = new Ember.RSVP.Promise(function(resolve) { + let resolveRequestWith; + const request = new Ember.RSVP.Promise(function(resolve) { resolveRequestWith = resolve; }); - var controller = this.subject(); - var viewSpy = { - showDropdownBySelector: sinon.spy() - }; + const currentUser = Discourse.User.create({ unread_notifications: 1}); + const controller = this.subject({ currentUser: currentUser }); + const viewSpy = { showDropdownBySelector: sinon.spy() }; + sandbox.stub(Discourse, "ajax").withArgs("/notifications").returns(request); - sandbox.stub(Discourse.User, "current").returns(Discourse.User.create({ - unread_notifications: 1 - })); Ember.run(function() { controller.send("showNotifications", viewSpy); }); equal(controller.get("notifications"), null, "notifications are null before data has finished loading"); - equal(Discourse.User.current().get("unread_notifications"), 1, "current user's unread notifications count is not zeroed before data has finished loading"); + equal(currentUser.get("unread_notifications"), 1, "current user's unread notifications count is not zeroed before data has finished loading"); ok(viewSpy.showDropdownBySelector.calledWith("#user-notifications"), "dropdown with loading glyph is shown before data has finished loading"); Ember.run(function() { @@ -31,6 +28,6 @@ test("showNotifications action", function() { // Can't use deepEquals because controller.get("notifications") is an ArrayProxy, not an Array ok(controller.get("notifications").indexOf("notification") !== -1, "notification is in the controller"); - equal(Discourse.User.current().get("unread_notifications"), 0, "current user's unread notifications count is zeroed after data has finished loading"); + equal(currentUser.get("unread_notifications"), 0, "current user's unread notifications count is zeroed after data has finished loading"); ok(viewSpy.showDropdownBySelector.calledWith("#user-notifications"), "dropdown with notifications is shown after data has finished loading"); }); diff --git a/test/javascripts/controllers/notifications-test.js.es6 b/test/javascripts/controllers/notifications-test.js.es6 deleted file mode 100644 index ed87b1d43f3..00000000000 --- a/test/javascripts/controllers/notifications-test.js.es6 +++ /dev/null @@ -1,7 +0,0 @@ -moduleFor('controller:notifications', 'controller:notifications', { - needs: ['controller:header'] -}); - -test("mixes in HasCurrentUser", function() { - ok(Discourse.HasCurrentUser.detect(this.subject())); -}); diff --git a/test/javascripts/controllers/site-map-test.js.es6 b/test/javascripts/controllers/site-map-test.js.es6 index 324a8b88cf7..f328d421379 100644 --- a/test/javascripts/controllers/site-map-test.js.es6 +++ b/test/javascripts/controllers/site-map-test.js.es6 @@ -13,26 +13,21 @@ moduleFor("controller:site-map", "controller:site-map", { }); test("showAdminLinks", function() { - var currentUserStub = Ember.Object.create(); - sandbox.stub(Discourse.User, "current").returns(currentUserStub); - - currentUserStub.set("staff", true); - var controller = this.subject(); + const currentUser = Ember.Object.create({ staff: true }); + const controller = this.subject({ currentUser }); equal(controller.get("showAdminLinks"), true, "is true when current user is a staff member"); - currentUserStub.set("staff", false); + currentUser.set("staff", false); equal(controller.get("showAdminLinks"), false, "is false when current user is not a staff member"); }); test("flaggedPostsCount", function() { - var currentUserStub = Ember.Object.create(); - sandbox.stub(Discourse.User, "current").returns(currentUserStub); + const currentUser = Ember.Object.create({ site_flagged_posts_count: 5 }); + const controller = this.subject({ currentUser }); - currentUserStub.set("site_flagged_posts_count", 5); - var controller = this.subject(); equal(controller.get("flaggedPostsCount"), 5, "returns current user's flagged posts count"); - currentUserStub.set("site_flagged_posts_count", 0); + currentUser.set("site_flagged_posts_count", 0); equal(controller.get("flaggedPostsCount"), 0, "is bound (reacts to change of current user's flagged posts count)"); }); diff --git a/test/javascripts/controllers/user-dropdown-test.js.es6 b/test/javascripts/controllers/user-dropdown-test.js.es6 index e45dca0ae71..6e34107283f 100644 --- a/test/javascripts/controllers/user-dropdown-test.js.es6 +++ b/test/javascripts/controllers/user-dropdown-test.js.es6 @@ -1,23 +1,19 @@ moduleFor("controller:user-dropdown"); test("logout action logs out the current user", function () { - var logout_mock = sinon.mock(Discourse, "logout"); - logout_mock.expects("logout").once(); + const logoutMock = sinon.mock(Discourse, "logout"); + logoutMock.expects("logout").once(); - var controller = this.subject(); - controller.send("logout"); + this.subject().send('logout'); - logout_mock.verify(); + logoutMock.verify(); }); test("showAdminLinks", function() { - var currentUserStub = Ember.Object.create(); - sandbox.stub(Discourse.User, "current").returns(currentUserStub); - - currentUserStub.set("staff", true); - var controller = this.subject(); + const currentUser = Ember.Object.create({ staff: true }); + const controller = this.subject({ currentUser }); equal(controller.get("showAdminLinks"), true, "is true when current user is a staff member"); - currentUserStub.set("staff", false); + currentUser.set("staff", false); equal(controller.get("showAdminLinks"), false, "is false when current user is not a staff member"); }); diff --git a/test/javascripts/mixins/has-current-user-test.js.es6 b/test/javascripts/mixins/has-current-user-test.js.es6 deleted file mode 100644 index 8f305731895..00000000000 --- a/test/javascripts/mixins/has-current-user-test.js.es6 +++ /dev/null @@ -1,12 +0,0 @@ -module("Discourse.HasCurrentUser"); - -test("adds `currentUser` property to an object and ensures it is not cached", function() { - sandbox.stub(Discourse.User, "current"); - var testObj = Ember.Object.createWithMixins(Discourse.HasCurrentUser, {}); - - Discourse.User.current.returns("first user"); - equal(testObj.get("currentUser"), "first user", "on the first call property returns initial user"); - - Discourse.User.current.returns("second user"); - equal(testObj.get("currentUser"), "second user", "if the user changes, on the second call property returns changed user"); -});