Set last_acknowledged_version automatically for new users (#33807)

Set `last_acknowledged_version` automatically for new users

This prevents new users from being told about what's new in Metabase
when…everything is new in Metabase for them

e2e/test/scenarios/onboarding/navbar/whats-new.cy.spec.js

@@ -25,6 +25,7 @@ describe("nav > what's new notification", () => {
 
   it("should show a notification with a link to the release notes, and allow the dismissal of it", () => {
     cy.signInAsAdmin();
+    cy.request("PUT", "api/setting/last-acknowledged-version", { value: null });
 
     loadHomepage();
     navigationSidebar().findByText("See what's new");
@@ -42,6 +43,7 @@ describe("nav > what's new notification", () => {
 
   it("it should show the notification for other users after one user dismissed it", () => {
     cy.signInAsAdmin();
+    cy.request("PUT", "api/setting/last-acknowledged-version", { value: null });
     loadHomepage();
     navigationSidebar().findByText("See what's new");
     navigationSidebar().icon("close").click();

src/metabase/models/user.clj

@@ -2,6 +2,8 @@
   (:require
    [clojure.data :as data]
    [clojure.string :as str]
+   [metabase.api.common :as api]
+   [metabase.config :as config]
    [metabase.db.query :as mdb.query]
    [metabase.integrations.common :as integrations.common]
    [metabase.models.collection :as collection]
@@ -13,7 +15,7 @@
     :refer [PermissionsGroupMembership]]
    [metabase.models.serialization :as serdes]
    [metabase.models.session :refer [Session]]
-   [metabase.models.setting :refer [defsetting]]
+   [metabase.models.setting :as setting :refer [defsetting]]
    [metabase.plugins.classloader :as classloader]
    [metabase.public-settings :as public-settings]
    [metabase.public-settings.premium-features :as premium-features]
@@ -70,6 +72,16 @@
       {:password_salt salt
        :password      (u.password/hash-bcrypt (str salt password))})))
 
+(defn user-local-settings
+  "Returns the user's settings (defaulting to an empty map) or `nil` if the user/user-id isn't set"
+  [user-or-user-id]
+  (when user-or-user-id
+    (or
+     (if (integer? user-or-user-id)
+       (:settings (t2/select-one [User :settings] :id user-or-user-id))
+       (:settings user-or-user-id))
+     {})))
+
 (t2/define-before-insert :model/User
   [{:keys [email password reset_token locale], :as user}]
   ;; these assertions aren't meant to be user-facing, the API endpoints should be validation these as well.
@@ -93,6 +105,12 @@
 (t2/define-after-insert :model/User
   [{user-id :id, superuser? :is_superuser, :as user}]
   (u/prog1 user
+    (let [current-version (:tag config/mb-version-info)]
+      (log/info (trs "Setting User {0}''s last_acknowledged_version to {1}, the current version" user-id current-version))
+      ;; Can't use mw.session/with-current-user-id due to circular require
+      (binding [api/*current-user-id*       user-id
+                setting/*user-local-values* (delay (atom (user-local-settings user)))]
+        (setting/set! :last-acknowledged-version current-version)))
     ;; add the newly created user to the magic perms groups.
     (log/info (trs "Adding User {0} to All Users permissions group..." user-id))
     (when superuser?

src/metabase/server/middleware/session.clj

@@ -296,11 +296,6 @@
   (when user-id
     (t2/select-one current-user-fields, :id user-id)))
 
-(defn- user-local-settings [user-id]
-  (when user-id
-    (or (:settings (t2/select-one [User :settings] :id user-id))
-        {})))
-
 (def ^:private ^:dynamic *user-local-values-user-id*
   "User ID that we've previous bound [[*user-local-values*]] for. This exists so we can avoid rebinding it in recursive
   calls to [[with-current-user]] if it is already bound, as this can mess things up since things
@@ -326,7 +321,7 @@
             *user-local-values*            (if (= *user-local-values-user-id* metabase-user-id)
                                              *user-local-values*
                                              (delay (atom (or settings
-                                                              (user-local-settings metabase-user-id)))))
+                                                              (user/user-local-settings metabase-user-id)))))
             *user-local-values-user-id*    metabase-user-id]
     (thunk)))
 

test/metabase/models/user_test.clj

@@ -18,6 +18,7 @@
    [metabase.models.setting :as setting]
    [metabase.models.user :as user]
    [metabase.public-settings.premium-features-test :as premium-features-test]
+   [metabase.server.middleware.session :as mw.session]
    [metabase.test :as mt]
    [metabase.test.data.users :as test.users]
    [metabase.test.integrations.ldap :as ldap.test]
@@ -513,14 +514,23 @@
                                               (salt)
                                               new-hashed-password)))))))))
 
-(deftest has-a-last-acknowledged-version
+(deftest last-acknowledged-version-can-be-read-and-set
   (testing "last-acknowledged-version can be read and set"
     (mt/with-test-user :rasta
-      (try
-        (is (nil? (setting/get :last-acknowledged-version)))
-        (setting/set! :last-acknowledged-version "47")
-        (is (= "47" (setting/get :last-acknowledged-version)))
-        ;; Ensure it's saved on the user, not globally:
-        (is (= "47" (:last-acknowledged-version (t2/select-one-fn :settings User :id (mt/user->id :rasta)))))
-        (finally
-          (setting/set! :last-acknowledged-version nil))))))
+      (let [old-version (setting/get :last-acknowledged-version)
+            new-version "v0.47.1"]
+        (try
+          (is (not= new-version old-version))
+          (setting/set! :last-acknowledged-version new-version)
+          (is (= new-version (setting/get :last-acknowledged-version)))
+          ;; Ensure it's saved on the user, not globally:
+          (is (= new-version (:last-acknowledged-version (t2/select-one-fn :settings User :id (mt/user->id :rasta)))))
+          (finally
+            (setting/set! :last-acknowledged-version old-version)))))))
+
+(deftest last-acknowledged-version-is-set-on-create
+  (testing "last-acknowledged-version is automatically set for new users"
+    (with-redefs [config/mb-version-info (assoc config/mb-version-info :tag "v0.47.1")]
+      (t2.with-temp/with-temp [User {user-id :id} {}]
+        (mw.session/with-current-user user-id
+          (is (= "v0.47.1" (setting/get :last-acknowledged-version))))))))