Преглед изворни кода

Fix the theme picker's login seeding and post-save re-render

Two real bugs the theme bundle's strengthened assertions caught:

The login seed never fired. SpryAuth.current() reads the request's
HELD slots - the frames the browser sent - while the auth slot this
login establishes rides the response, so inside LoginAction the
current user was always anonymous and the seed silently no-op'd. The
credentials are now resolved once more through the user service after
base.handle() and the stored preferences seed the DEVICE ui slot on
the same directive chain (a storage failure still degrades to a
warning).

The save snapped the radios back. The client re-renders every binding
after a directive chain, so SetThemeAction answering only the ui slot
re-evaluated the theme radios against the STALE page profile slot.
The action now also answers the refreshed PAGE profile slot through a
shared ProfileEntrypoint.build_profile_slot (the dashboard-action
pattern). 'core' normalises to the canonical empty slug so no
data-theme that scopes nothing is ever stored.
saddle-ClankerSaddle пре 1 дан
родитељ
комит
32ad3ba68e

+ 25 - 14
src/actions/LoginAction.vala

@@ -9,33 +9,44 @@ namespace ClankerSaddle {
     /**
      * The shipped {@link Spry.Actions.LoginAction} with this app's
      * post-login destination (the dashboard) and its appearance
-     * seeding (goal 41 task 2): after Spry establishes the auth
-     * session, the user's stored theme/mode preferences are written
-     * to the DEVICE-scope `ui` slot, so the client's theme hook
-     * paints the chosen look from the very first page and keeps it
-     * cached on this device across sessions.
+     * seeding (goal 41 task 2): a successful login also writes the
+     * user's stored theme/mode preferences to the DEVICE-scope `ui`
+     * slot, so the client's theme hook paints the chosen look from
+     * the very first page and keeps it cached on this device across
+     * sessions.
+     *
+     * The user id CANNOT come from {@link SpryAuth.current} here: the
+     * request's held slots are the ones the browser SENT, and the auth
+     * slot this login establishes rides the RESPONSE — so the
+     * credentials are resolved once more through the user service
+     * (a failed login short-circuits to Spry's own answer, and a
+     * storage failure degrades to a warning: the login itself must
+     * not break).
      */
     public class LoginAction : Spry.Actions.LoginAction {
 
         protected override string landing_uri { get { return "/"; } }
 
+        private UserService users = inject<UserService>();
         private UserPreferenceService preferences = inject<UserPreferenceService>();
 
         public override async DirectiveBuilder handle() throws GLib.Error {
             var landing = yield base.handle();
 
-            // The auth slot only exists on a successful login (a
-            // failed one answers a notify-only builder); seed the
-            // appearance slot from the stored preferences on the SAME
-            // directive chain — a storage failure must not break the
-            // login itself.
-            var me = SpryAuth.current(request);
-            if (me.is_anonymous) {
+            string username = "";
+            string password = "";
+            if (request.form != null) {
+                username = request.form.get_field("username") ?? "";
+                password = request.form.get_field("password") ?? "";
+            }
+            var user = yield users.authenticate_user(username, password);
+            if (user == null) {
                 return landing;
             }
+
             try {
-                string theme = yield preferences.get_theme(me.user_id);
-                string mode = yield preferences.get_mode(me.user_id);
+                string theme = yield preferences.get_theme(((!)user).id);
+                string mode = yield preferences.get_mode(((!)user).id);
                 var ui = new UiSlotPublic() { theme = theme, mode = mode };
                 return landing.set_typed<UiSlotPublic>(SetThemeAction.UI_SLOT_TYPE,
                     Statum.Scope.DEVICE, ui);

+ 21 - 3
src/actions/ProfileActions.vala

@@ -55,6 +55,7 @@ namespace ClankerSaddle {
 
         private UserService users = inject<UserService>();
         private UserPreferenceService preferences = inject<UserPreferenceService>();
+        private StateService states = inject<StateService>();
 
         /** True when `slug` names a theme the client may apply. */
         public static bool is_valid_theme(string slug) {
@@ -90,6 +91,12 @@ namespace ClankerSaddle {
             if (mode.length == 0) {
                 mode = MODE_DEFAULT;
             }
+            // "core" is the same choice as "" (the default); store the
+            // canonical empty slug so the client never carries a
+            // data-theme that scopes no stylesheet.
+            if (theme == "core") {
+                theme = "";
+            }
 
             if (!is_valid_theme(theme)) {
                 return directives().notify("error",
@@ -104,9 +111,20 @@ namespace ClankerSaddle {
             try {
                 yield preferences.set_theme(me.user_id, theme);
                 yield preferences.set_mode(me.user_id, mode);
-                var slot = new UiSlotPublic() { theme = theme, mode = mode };
-                return directives()
-                    .set_typed<UiSlotPublic>(UI_SLOT_TYPE, Statum.Scope.DEVICE, slot)
+                var ui = new UiSlotPublic() { theme = theme, mode = mode };
+                var builder = directives()
+                    .set_typed<UiSlotPublic>(UI_SLOT_TYPE, Statum.Scope.DEVICE, ui);
+                // Answer with the refreshed profile card too (the
+                // dashboard-action pattern): the client re-renders
+                // every binding after the chain, and without this the
+                // radios would snap back to the pre-save values.
+                var user = yield users.get_user(me.user_id);
+                if (user != null) {
+                    var profile = yield ProfileEntrypoint.build_profile_slot(
+                        states, action_registry, users, preferences, me, (!)user);
+                    builder.set(profile);
+                }
+                return builder
                     .notify("success", "Appearance saved — the new theme applies immediately");
             } catch (Error e) {
                 return directives().notify("error",

+ 23 - 7
src/entrypoints/ProfileEntrypoint.vala

@@ -51,6 +51,23 @@ namespace ClankerSaddle {
                 return directives().notify("error", "Your account no longer exists.").navigate("/login");
             }
 
+            var slot = yield build_profile_slot(state_service, action_registry,
+                users_service, preferences, me, (!)user);
+            return directives().set(slot);
+        }
+
+        /**
+         * The page's PAGE-scope profile slot. Shared with
+         * {@link SetThemeAction} so the save answers with the REFRESHED
+         * card (the same trick as the dashboard actions): the client
+         * re-renders every binding after a directive chain, and without
+         * this the theme radios would snap back against the stale
+         * pre-save values.
+         */
+        public static async Statum.Slot build_profile_slot(StateService state_service,
+                ActionRegistry action_registry, UserService users_service,
+                UserPreferenceService preferences, CurrentUser me, UserProjection user)
+                throws GLib.Error {
             // The appearance card's current values: the stored
             // preferences (missing rows read as the defaults — the
             // core theme, auto mode).
@@ -64,12 +81,12 @@ namespace ClankerSaddle {
             }
 
             var view = new ProfileView() {
-                username = ((!)user).username,
-                email = ((!)user).email,
-                name_display = @"$(((!)user).forename) $(((!)user).surname)".strip(),
-                is_admin = SpryAuth.permission_matches(((!)user).permissions.to_array(),
+                username = user.username,
+                email = user.email,
+                name_display = @"$(user.forename) $(user.surname)".strip(),
+                is_admin = SpryAuth.permission_matches(user.permissions.to_array(),
                     SpryAuth.ADMIN_PERMISSION),
-                created_display = format_stamp(((!)user).created),
+                created_display = format_stamp(user.created),
                 change_password = action_registry.author<ChangeMyPasswordAction>(),
                 theme = theme,
                 mode = mode,
@@ -78,12 +95,11 @@ namespace ClankerSaddle {
 
             var props = (PropertyDictionary) GObjectMapping.to_properties(view);
 
-            var slot = state_service.new_slot(Scope.PAGE, new State() {
+            return state_service.new_slot(Scope.PAGE, new State() {
                 type_name = "profile",
                 public_data = props,
                 private_data = new PropertyDictionary()
             });
-            return directives().set(slot);
         }
     }
 }

+ 21 - 16
test/bundles/theme.py

@@ -10,6 +10,18 @@ from harness import *  # noqa: F401,F403
 def run(ctx):
     admin = ctx.auth
 
+    def login_frames(username, password):
+        """The login action's FULL directive chain (the harness login()
+        only keeps the auth slot; the ui seed rides the same chain)."""
+        frames = directives("/login")
+        content = json.loads(frames[0]["snapshot_frame"]["content"])
+        action = content["public"]["login"]
+        status, body = http(action.get("method", "POST"), action["uri"],
+                            {"username": username, "password": password})
+        if status != 200:
+            raise SystemExit("login -> HTTP %d: %s" % (status, body[:300]))
+        return json.loads(body)
+
     print("== profile page: the appearance card ==")
     profile = slot_public(admin, "/profile", "profile")
     check(profile.get("set_theme") is not None,
@@ -99,21 +111,14 @@ def run(ctx):
           "profile reads the core default back")
 
     print("== login seeds the ui slot from stored prefs ==")
-    fresh = login("admin", "adminpass123")
-    seeded = [d for d in directives("/", fresh)
+    # The seed rides the LOGIN response's directive chain (the same
+    # builder Spry's login action returns, extended by our subclass) -
+    # so assert it there, not on a later entrypoint.
+    frames = login_frames("admin", "adminpass123")
+    seeded = [d for d in frames
               if d["type"] == "set"
               and json.loads(d["snapshot_frame"]["content"])["slot"]["type"] == "ui"]
-    # The DEVICE slot is client-held; a fresh login (a browser with no
-    # cached frame) gets it seeded on the same directive chain as the
-    # auth slot.
-    if seeded:
-        seeded_public = json.loads(seeded[0]["snapshot_frame"]["content"])["public"]
-        check(seeded_public.get("theme") == "" and seeded_public.get("mode") == "light",
-              "login seeds the ui slot from the stored preferences")
-    else:
-        # The entrypoint chain does not re-emit a held DEVICE slot when
-        # the request already carries one; assert the seed indirectly
-        # through the profile's mirror of the stored values instead.
-        profile = slot_public(fresh, "/profile", "profile")
-        check(profile.get("theme") == "" and profile.get("mode") == "light",
-              "stored preferences survive a re-login (profile mirror)")
+    check(len(seeded) == 1, "the login response answers a ui slot")
+    seeded_public = json.loads(seeded[0]["snapshot_frame"]["content"])["public"]
+    check(seeded_public.get("theme") == "" and seeded_public.get("mode") == "light",
+          "login seeds the ui slot from the stored preferences")