Prechádzať zdrojové kódy

Stop the wizard offering to omit a title, a visibility or acls

Every post has all three, so a post type does not get to decide whether its
editor collects them. The three checkboxes are gone and post_wizard_save forces
the includes, which keeps them in their usual position in the generated form
and recorded in the sidecar.

This was not hypothetical: the hypervisor type was built without the visibility
include, so nothing ever set one, and _process pushed an undef into the tags --
a tag no query matches, which quietly made the post invisible to everyone but
an admin. hypervisor.tx is regenerated here with all three.

Also fixes the other half of why that series was empty for logged out
visitors. posts() widens user_acls to include 'public', but series() resolves
the series' own relations before that runs, so an anonymous visitor resolved
them against an empty acl list and found nothing however public the target
was. The relations a datasource reads are exactly the ones resolved there, so
this is the difference between a guests page listing your infrastructure and
one 404ing.

The test for it asserts on the primary post's resolved relation rather than on
the rendered children: children get their relations resolved later in posts(),
by a path that was never broken, so an assertion on the rendered output passes
whether or not the bug is present. It does now fail when the fix is reverted,
which the first version of it did not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U3k2hhSZpcsoe6uBmhF9nB
George Baugh 2 mesiacov pred
rodič
commit
a596211451

+ 31 - 5
lib/Trog/Routes/HTML.pm

@@ -1184,7 +1184,15 @@ sub series ($query) {
             # only the relation keys back, so nothing else about the series
             # post gets overwritten.
             my $stand_in = { %{ $posts[0] }, form => $posts[0]{child_form} };
-            _enrich_post( $stand_in, $query );
+
+            # posts() widens user_acls to what a viewer can actually see, but
+            # that happens after this -- so resolving here against the raw list
+            # means a logged out visitor resolves every relation against no
+            # acls at all, and finds nothing however public the target is.
+            my @acls = ( @{ $query->{user_acls} }, 'public' );
+            push( @acls, 'private' ) if $is_admin;
+
+            _enrich_post( $stand_in, { %$query, user_acls => \@acls } );
             @{ $posts[0] }{ keys(%$relations) } = @{$stand_in}{ keys(%$relations) };
         }
     }
@@ -2009,8 +2017,17 @@ our %wizard_includes = (
 our @wizard_include_order = qw{inc_preview inc_visibility inc_acls inc_tags inc_aliases inc_attachments};
 
 # Every checkbox on the wizard, and which of them start out ticked.
-our @wizard_checkboxes         = ( qw{wrapper inc_post_title inc_post_tags inc_title_input}, @wizard_include_order, 'overwrite' );
-our %wizard_checked_by_default = map { $_ => 1 } qw{wrapper inc_post_title inc_post_tags inc_title_input inc_preview inc_visibility inc_acls inc_tags inc_aliases};
+# Every post has a title, a visibility and a set of acls, so a type does not
+# get to decide whether its editor collects them -- see post_wizard_save.
+our @wizard_mandatory = qw{inc_title_input inc_visibility inc_acls};
+
+our @wizard_optional_includes = grep {
+    my $include = $_;
+    !grep { $include eq $_ } @wizard_mandatory
+} @wizard_include_order;
+
+our @wizard_checkboxes         = ( qw{wrapper inc_post_title inc_post_tags}, @wizard_optional_includes, 'overwrite' );
+our %wizard_checked_by_default = map { $_ => 1 } qw{wrapper inc_post_title inc_post_tags inc_preview inc_tags inc_aliases};
 
 # Which HTML input the wizard emits, and what that field is in the sidecar's
 # OpenAPIv3 schema.  No 'file': _process() in Trog::DataModule only knows how to
@@ -2186,8 +2203,17 @@ sub post_wizard_save ($query) {
     return _wizard_fail( $query, "Post type '$name.tx' already exists.  Tick 'Overwrite' if you meant to replace it." )
       if $overwriting && !$query->{overwrite};
 
-    my $fields    = _wizard_fields($query);
-    my @includes  = grep { $query->{$_} } @wizard_include_order;
+    my $fields = _wizard_fields($query);
+
+    # Not up for discussion.  A post with no visibility gets an undef pushed
+    # into its tags by _process, which is a tag nothing matches -- so the post
+    # is invisible to everyone but an admin, for no reason anybody chose.  A
+    # title and a set of acls are equally non-negotiable: every other form has
+    # them, and a type whose editor cannot set them produces posts nobody can
+    # title or restrict.
+    $query->{$_} = 1 foreach @wizard_mandatory;
+
+    my @includes = grep { $query->{$_} } @wizard_include_order;
     my $body_form = _wizard_scalar( $query->{body_form} ) eq 'form_multi.tx' ? 'form_multi.tx' : 'form_common.tx';
 
     my $spec = _wizard_sidecar( $name, $fields, \@includes, $body_form );

+ 30 - 0
t/Trog-Routes-HTML-series-lifecycle.t

@@ -435,6 +435,18 @@ subtest 'the post wizard invents a new post type' => sub {
     my $wrapper = q{<div class="post <: $style :> <: $tiled ? 'tile' : '' :>">};
     ok( index( $generated, $wrapper ) >= 0, 'the generated wrapper carries the tile class when the series asks for it' )
       or diag( "wanted: $wrapper\ngot:    " . ( split( /\n/, $generated ) )[1] );
+
+    # Every post has a title, a visibility and acls.  The wizard used to let a
+    # type opt out of collecting them, which is how a hypervisor post ended up
+    # stored with no visibility and an undef in its tags.
+    foreach my $required ( 'visibility.tx', 'acls.tx' ) {
+        like( $generated, qr/\Q: include "$required";\E/, "$required is included whether or not it was asked for" );
+    }
+    like( $generated, qr/name="title"/, 'and so is the title input' );
+
+    foreach my $gone (qw{inc_title_input inc_visibility inc_acls}) {
+        unlike( $body, qr/name="\Q$gone\E"/, "the wizard no longer offers '$gone' as a choice" );
+    }
 };
 
 #--------------------------------------------------------------------------
@@ -850,6 +862,24 @@ subtest '/api/posts_of_form feeds the relation picker' => sub {
     ok( $Trog::Routes::JSON::routes{'/api/posts_of_form'}{auth}, 'the route requires a login' );
 };
 
+subtest 'a series resolves its relations for logged out visitors too' => sub {
+
+    # The relations series() resolves land on the primary post, which is what a
+    # datasource reads to find its hypervisors -- so assert on that rather than
+    # on the rendered children, whose relations posts() resolves later, after
+    # it has widened user_acls and thus by a path that was never broken.
+    foreach my $case ( [ 'anonymously', _anon( route => '/specrelator' ) ], [ 'as an admin', _admin( route => '/specrelator' ) ] ) {
+        my ( $label, $query ) = @$case;
+
+        my ( $code, undef, $err ) = _render( $query, \&Trog::Routes::HTML::series );
+        is( $code, 200, "the series renders $label" ) or do { diag($err); next };
+
+        my $resolved = $query->{primary_post}{everything};
+        is( ref $resolved, 'ARRAY', "$label, the series' own relation resolved" );
+        cmp_ok( scalar @{ $resolved || [] }, '>', 0, "$label, to something rather than nothing" );
+    }
+};
+
 subtest 'saves report back through the jsalert banner' => sub {
 
     # A good save redirects to where you came from, carrying the outcome.

+ 1 - 0
www/templates/html/components/forms/hypervisor.json

@@ -16,6 +16,7 @@
       "generated" : true,
       "generator" : "Trog::Routes::HTML::post_wizard_save",
       "includes" : [
+         "visibility.tx",
          "acls.tx",
          "tags.tx",
          "aliases.tx"

+ 2 - 1
www/templates/html/components/forms/hypervisor.tx

@@ -1,5 +1,5 @@
 <!-- Generated by the tCMS Post Type Wizard.  Regenerating this type will overwrite any hand edits. -->
-<div class="post <: $style :>">
+<div class="post <: $style :> <: $tiled ? 'tile' : '' :>">
     : if ( !$post.addpost ) {
         : include "post_title.tx";
         : include "post_tags.tx";
@@ -17,6 +17,7 @@
         <form class="Submissions" action="/post/save" method="POST" enctype="multipart/form-data">
             Title *<br /><input required class="cooltext" type="text" name="title" placeholder="HV Hostname" value="<: $post.title :>" />
             Libvirt Connection URI<br /><input required class="cooltext" type="url" name="conn_uri" placeholder="libvirt://localhost" value="<: $post.conn_uri :>" />
+            : include "visibility.tx";
             : include "acls.tx";
             : include "tags.tx";
             : include "aliases.tx";

+ 3 - 3
www/templates/html/components/post_wizard.tx

@@ -27,14 +27,14 @@ any custom fields, so that the data model knows to keep them when a post is save
 
     <p class="title">Canned Fields</p>
     Most post types reuse the same handful of blocks.  Tick whichever ones this type needs.
+    The title input, the visibility selector (visibility.tx) and the ACL selector (acls.tx)
+    are always included &mdash; every post has all three, and a type whose editor cannot set
+    them produces posts nobody can title, find or restrict.
     <br />
     <label for="wiz-wrapper">Wrap output in the standard post &lt;div&gt;<input id="wiz-wrapper" class="coolcb" type="checkbox" name="wrapper" value="1" <: $checked.wrapper :> /></label><br />
     <label for="wiz-post_title">Post title &amp; permalink (post_title.tx)<input id="wiz-post_title" class="coolcb" type="checkbox" name="inc_post_title" value="1" <: $checked.inc_post_title :> /></label><br />
     <label for="wiz-post_tags">Post tag list (post_tags.tx)<input id="wiz-post_tags" class="coolcb" type="checkbox" name="inc_post_tags" value="1" <: $checked.inc_post_tags :> /></label><br />
-    <label for="wiz-title_input">Title input in the editor<input id="wiz-title_input" class="coolcb" type="checkbox" name="inc_title_input" value="1" <: $checked.inc_title_input :> /></label><br />
     <label for="wiz-preview">Preview image upload (preview.tx)<input id="wiz-preview" class="coolcb" type="checkbox" name="inc_preview" value="1" <: $checked.inc_preview :> /></label><br />
-    <label for="wiz-visibility">Visibility selector (visibility.tx)<input id="wiz-visibility" class="coolcb" type="checkbox" name="inc_visibility" value="1" <: $checked.inc_visibility :> /></label><br />
-    <label for="wiz-acls">ACL selector (acls.tx)<input id="wiz-acls" class="coolcb" type="checkbox" name="inc_acls" value="1" <: $checked.inc_acls :> /></label><br />
     <label for="wiz-tags">Tag editor (tags.tx)<input id="wiz-tags" class="coolcb" type="checkbox" name="inc_tags" value="1" <: $checked.inc_tags :> /></label><br />
     <label for="wiz-aliases">Alias URL editor (aliases.tx)<input id="wiz-aliases" class="coolcb" type="checkbox" name="inc_aliases" value="1" <: $checked.inc_aliases :> /></label><br />
     <label for="wiz-attachments">Attachment uploader (attachments.tx)<input id="wiz-attachments" class="coolcb" type="checkbox" name="inc_attachments" value="1" <: $checked.inc_attachments :> /></label><br />