| Summary: | Edit basket vendor after it has been created | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Kyle M Hall (khall) <kyle> |
| Component: | Acquisitions | Assignee: | Kyle M Hall (khall) <kyle> |
| Status: | CLOSED FIXED | QA Contact: | Paul Poulain <paul.poulain> |
| Severity: | enhancement | ||
| Priority: | P3 | CC: | chris, jonathan.druart, paul.poulain, veron |
| Version: | 3.10 | ||
| Hardware: | All | ||
| OS: | All | ||
| GIT URL: | Initiative type: | --- | |
| Sponsorship status: | --- | Comma delimited list of Sponsors: | |
| Crowdfunding goal: | 0 | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: | ||
| Circulation function: | |||
| Bug Depends on: | |||
| Bug Blocks: | 8247, 25611 | ||
| Attachments: |
Bug 7757 - Edit basket vendor after it has been created
[SIGNED-OFF] Bug 7757 - Edit basket vendor after it has been created Bug 7757 - Followup - Move IF outside of html tag Bug 7757 - Followup - Move IF outside of html tag Bug 7757 - Followup - Move IF outside of html tag Bug 7757 - Followup - Move IF outside of html tag |
||
|
Description
Kyle M Hall (khall)
2012-03-20 13:22:40 UTC
Created attachment 8614 [details] [review] Bug 7757 - Edit basket vendor after it has been created Created attachment 8615 [details] [review] [SIGNED-OFF] Bug 7757 - Edit basket vendor after it has been created Signed-off-by: mveron <veron@veron.ch> Works as expected. With patch applied I have an additional drop down where I can choose th vendor in Edit basket and can change vendor as appropiate. Template [% IF %] are not permissible inside HTML tags, as they can break translations. xt/tt_valid.t fails. Marking Failed QA. Created attachment 9391 [details] [review] Bug 7757 - Followup - Move IF outside of html tag Hi Kyle, It seems you have unfortunately deleted a 'o' character in your patch :) Your patch replaces the zoom css properties with 'zom' (which does not exist). Created attachment 9422 [details] [review] Bug 7757 - Followup - Move IF outside of html tag Fixed, thanks for the heads up. (In reply to comment #6) > Hi Kyle, > > It seems you have unfortunately deleted a 'o' character in your patch :) > Your patch replaces the zoom css properties with 'zom' (which does not > exist). Created attachment 9424 [details] [review] Bug 7757 - Followup - Move IF outside of html tag Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz> QA comment:
there's something strange when I apply the 2nd patch : I get:
[% FOREACH b IN booksellers %]
llers %]
[% IF booksellerid == b.id %]
b.id %]
<option value="[% b.id %]" selected="selected">[% b.name %]</option>
/option>
[% ELSE %]
ELSE %]
<option value="[% b.id %]">[% b.name %]</option>
/option>
(yes, truncated code) does others have the same problem ?
Created attachment 9588 [details] [review] Bug 7757 - Followup - Move IF outside of html tag Paul, try with it (I removed tabulation characters) (In reply to comment #11) > Created attachment 9588 [details] [review] > Bug 7757 - Followup - Move IF outside of html tag > > Paul, try with it (I removed tabulation characters) Not better I've rewritten the patch QA comment: * patch #2 = tiny patch, necessary, passed QA * patch #1 = * improve some code, like -my $booksellerid; -$booksellerid = $input->param('booksellerid'); +my $booksellerid = $input->param('booksellerid'); * coding guidelines OK * I'm not sure - ModBasketHeader($input->param('basketno'),$input->param('basketname'),$input->param('basketnote'),$input->param('basketbooksellernote'),$input->param('basketcontractnumber')); + ModBasketHeader( $input->param('basketno'), $input->param('basketname'), $input->param('basketnote'), $input->param('basketbooksellernote'), $input->param('basketcontractnumber') || undef, $input->param('basketbooksellerid') ); is the best way to deal with an empty basketcontractnumber ( $input->param('basketcontractnumber') || undef ), but there's nothing about that in the coding guidelines, so I won't object passed QA About parameters: I think we should define a rule for new subs, about how we pass parameters. I think passing hashes would gracefully solve this kind of problem. So what about a rule like = identifier/mandatory fields are passed directly, others are passed through a hash The ModBasketHeader would then become: ModBasketHeader($basketno, { basketname => $basketname, basketnote => $basketnote, basketbooksellernote => $basketbooksellernote, basketcontractnumber => basketcontractnumber, basketbooksellerid=> $basketbooksellerid} ); Kyle, if you agree/like this idea, I can start a discussion on koha-devel !
> About parameters: I think we should define a rule for new subs, about how we
> pass parameters. I think passing hashes would gracefully solve this kind of
> problem. So what about a rule like = identifier/mandatory fields are passed
> directly, others are passed through a hash
> The ModBasketHeader would then become:
> ModBasketHeader($basketno,
> { basketname => $basketname,
> basketnote => $basketnote,
> basketbooksellernote => $basketbooksellernote,
> basketcontractnumber => basketcontractnumber,
> basketbooksellerid=> $basketbooksellerid}
> );
>
> Kyle, if you agree/like this idea, I can start a discussion on koha-devel !
Paul, I am 110% behind this idea. I've been using hashes for parameter passing in most of my new code. I think it makes it much easier to extend subroutines after the fact. Right new we have subs that look like sub( $param1, $parm2, undef, $param3, 1, $param4 ) and this would make our code far better.
I just re-read your comment. Do you think it's really necessary to pass required params directly? I really like the passing a single hash as the param list like so:
mysub(
param1 => $param1,
param2 => $param2,
param3 => $param3,
);
mysub {
my %params = @_;
my $param1 = $params{'param1'};
my $param2 = $params{'param2'};
my $param3 = $params{'param3'};
return unless ( $param1 && $param2 )
## Do Stuff
}
(In reply to comment #13)
> > About parameters: I think we should define a rule for new subs, about how we
> > pass parameters. I think passing hashes would gracefully solve this kind of
> > problem. So what about a rule like = identifier/mandatory fields are passed
> > directly, others are passed through a hash
> > The ModBasketHeader would then become:
> > ModBasketHeader($basketno,
> > { basketname => $basketname,
> > basketnote => $basketnote,
> > basketbooksellernote => $basketbooksellernote,
> > basketcontractnumber => basketcontractnumber,
> > basketbooksellerid=> $basketbooksellerid}
> > );
> >
> > Kyle, if you agree/like this idea, I can start a discussion on koha-devel !
>
> Paul, I am 110% behind this idea. I've been using hashes for parameter
> passing in most of my new code. I think it makes it much easier to extend
> subroutines after the fact. Right new we have subs that look like sub(
> $param1, $parm2, undef, $param3, 1, $param4 ) and this would make our code
> far better.
This is especially better when a sub requires one of multiple options ( we have a number of subroutines that require either $param1 OR $param2, but not necessarily both ). New feature, held for 3.10 There have been no further reports of problems so I am marking this bug resolved. |