[Catalyst] Reaction Development?

[John Napiorkowski]

--- John Napiorkowski <jjn1056 at yahoo.com> wrote:

> 
> --- Jason Kohles <email at jasonkohles.com> wrote:
> 
> > On Feb 5, 2007, at 5:58 PM, John Napiorkowski
> wrote:
> > 
> > > Hi,
> > >
> > > Is there a test suite for Reaction other than
> the
> > > sample application at the source control site?
> > > Because I have a patch to fix a problem with the
> > DBIC
> > > action reflector not properly creating actions
> for
> > > DBIC classes in a deep hierarchy (like
> > > myschema::membership::members) but since I could
> > never
> > > get the sample app to run I can't write a test
> for
> > the
> > > problem (and I know a patch without a test won't
> > be
> > > accepted).
> > >
> > Any chance you could share the patch even though
> it
> > doesn't have  
> > tests yet?  I've been looking for this bug off and
> > on for the last  
> > three or four days, it's been driving me crazy...
> 
> Not sure if this is the same bug as what is causing
> you trouble.  My problem is when I have 'deep'
> hierarchies under my DBIC Schema, something like:
> 
> /myapplib
>   /Schema
>     db.pm, # inheriting from DBIx::Class::Schema
>       /db
>         /membership
>           members.pm #inherit from DBIx::Class
> 
> so the package name for that "members.pm" file would
> be something like:
> 
> package myapplib::Schema::db::membership::members;
> 
> I found the actions that the action reflector would
> create would look something like:
> 
> catapp::Model::Action::Createmembers                
> catapp::Model::Action::Deletemembers                
> catapp::Model::Action::Updatemembers 
> 
> Which was bad because then when I tried to access
> the
> create/delete/update actions I'd get an error, since
> it wasn't properly linked to my real model name
> space.
> 
> I made a very small change in
> ...DBIC::ActionReflector
> and got:
> 
> catapp::Model::Action::Createmembership::members    
>  
>          
> catapp::Model::Action::Deletemembership::members    
>  
>          
> catapp::Model::Action::Updatemembership::members    
>  
>          
> 
> which worked for me :)
> 
> I'm still not sure this is totally correct.  I think
> I'd personally prefer:
> 
> catapp::Model::Action::Create::membership::members  
>  
>            
> catapp::Model::Action::Delete::membership::members  
>  
>            
> catapp::Model::Action::Update::membership::members 
> 
> Instead, but I guess there's a good reason it's not
> this way.
> 
> One thing I'm not sure about is if I should regex to
> remove all the "::" to something like:
> 
> catapp::Model::Action::Createmembership-members     
>  
>         
> catapp::Model::Action::Deletemembership-members     
>  
>         
> catapp::Model::Action::Updatemembership-members 
> 
> I tried that and it still worked for me, but again I
> am not knowledgeable enough to know what is best.
>  
> Here's my patch; hope it helps, or at least starts
> to
> help (also attached):
> 
> Index: InterfaceModel/Action/DBIC/ActionReflector.pm
>
===================================================================
> --- InterfaceModel/Action/DBIC/ActionReflector.pm
> (revision 282)
> +++ InterfaceModel/Action/DBIC/ActionReflector.pm
> (working copy)
> @@ -37,8 +37,8 @@
>    implements reflect_actions_for => as {
>      my ($self, $class, $reflected_prefix) = @_;
>      foreach my $action ( keys %{
> $self->reflect_action_types } ) {
> -      my @stem_parts = split('::', $class);
> -      my $last_part = pop(@stem_parts);
> +	  my $base_schema =
> $class->result_source_instance->schema();
> +	  my ($last_part) =
> $class=~/$base_schema\:\:(.+?)$/;
>        my $action_class =
> "${reflected_prefix}::${action}${last_part}";
>        $self->reflect_action_for($class,
> $action_class, $action);
>      }
> 

Hi,

After looking at this I could see it would be better
to move the call to result_source_instance->schema()
to outside the loop and save some time.   I could
probably precompile the regex as well, but I doubt it
would help very much for this simple match.  Updated
patch attached.

--john




 
____________________________________________________________________________________
Do you Yahoo!?
Everyone is raving about the all-new Yahoo! Mail beta.
http://new.mail.yahoo.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: reaction.patch
Type: application/octet-stream
Size: 784 bytes
Desc: 2107148715-reaction.patch
Url : http://lists.scsys.co.uk/pipermail/catalyst/attachments/20070206/eaed8113/reaction-0001.obj