Login | Register For Free | Help
Search for: (Advanced)

Mailing List Archive: Bricolage: devel

Patch to fix accessors in Bric::Biz::Element::Field

 

 

Bricolage devel RSS feed   Index | Next | Previous | View Threaded


adrian at gossamer-threads

Feb 6, 2009, 10:05 AM

Post #1 of 8 (2475 views)
Permalink
Patch to fix accessors in Bric::Biz::Element::Field

Hi,

While figuring things out for the in-place editor, I ran into what seems like a
bug where Bric::Biz::Element::Field->get_max_length was returning $self instead
of the max_length. I took a look at the SVN log, but there wasn't a message
relating to why it was added:

http://viewsvn.bricolage.cc/bricolage/trunk/lib/Bric/Biz/Element/Field.pm?view=diff&r1=6698&r2=6699

Here's the patch, essentially removing the bit of code and relying on the built
in accessors:

***** start patch *****
Index: lib/Bric/Biz/Element/Field.pm
===================================================================
--- lib/Bric/Biz/Element/Field.pm (revision 8406)
+++ lib/Bric/Biz/Element/Field.pm (working copy)
@@ -731,12 +731,6 @@

=cut

-sub get_max_length {
- my $self = shift;
- return $self if $self->_get('max_length');
- return;
-}
-
##############################################################################

=item my $sql_type = $data->get_sql_type
***** end patch *****

There are a couple other methods which do the same thing that may want to be
removed/fixed as well: is_autopopulated, is_multiple

Adrian


david at kineticode

Feb 6, 2009, 12:46 PM

Post #2 of 8 (2353 views)
Permalink
Re: Patch to fix accessors in Bric::Biz::Element::Field [In reply to]

On Feb 6, 2009, at 10:05 AM, Adrian Yee wrote:

> Hi,
>
> While figuring things out for the in-place editor, I ran into what
> seems like a
> bug where Bric::Biz::Element::Field->get_max_length was returning
> $self instead
> of the max_length. I took a look at the SVN log, but there wasn't a
> message
> relating to why it was added:
>
> http://viewsvn.bricolage.cc/bricolage/trunk/lib/Bric/Biz/Element/Field.pm?view=diff&r1=6698&r2=6699
>
> Here's the patch, essentially removing the bit of code and relying
> on the built
> in accessors:
>
> ***** start patch *****
> Index: lib/Bric/Biz/Element/Field.pm
> ===================================================================
> --- lib/Bric/Biz/Element/Field.pm (revision 8406)
> +++ lib/Bric/Biz/Element/Field.pm (working copy)
> @@ -731,12 +731,6 @@
>
> =cut
>
> -sub get_max_length {
> - my $self = shift;
> - return $self if $self->_get('max_length');
> - return;
> -}
> -
> ##############################################################################
>
> =item my $sql_type = $data->get_sql_type
> ***** end patch *****
>
> There are a couple other methods which do the same thing that may
> want to be
> removed/fixed as well: is_autopopulated, is_multiple

Those should work that way; they're booleans.

Are you running the test suite as you do this stuff? See Bric::Hacker
for details. You should be adding new tests, too, eh?

Thanks,

David


david at kineticode

Mar 11, 2009, 10:32 AM

Post #3 of 8 (2206 views)
Permalink
Re: Patch to fix accessors in Bric::Biz::Element::Field [In reply to]

Hi Adrian,

Sorry, this fell through the cracks, though I know we did discuss it
when I reviewed your patch. Please do feel free to change these
methods, provided all tests continue to pass. Have you managed to get
tests passing yet?

Thanks,

David

On Feb 6, 2009, at 10:05 AM, Adrian Yee wrote:

> Hi,
>
> While figuring things out for the in-place editor, I ran into what
> seems like a
> bug where Bric::Biz::Element::Field->get_max_length was returning
> $self instead
> of the max_length. I took a look at the SVN log, but there wasn't a
> message
> relating to why it was added:
>
> http://viewsvn.bricolage.cc/bricolage/trunk/lib/Bric/Biz/Element/Field.pm?view=diff&r1=6698&r2=6699
>
> Here's the patch, essentially removing the bit of code and relying
> on the built
> in accessors:
>
> ***** start patch *****
> Index: lib/Bric/Biz/Element/Field.pm
> ===================================================================
> --- lib/Bric/Biz/Element/Field.pm (revision 8406)
> +++ lib/Bric/Biz/Element/Field.pm (working copy)
> @@ -731,12 +731,6 @@
>
> =cut
>
> -sub get_max_length {
> - my $self = shift;
> - return $self if $self->_get('max_length');
> - return;
> -}
> -
> ##############################################################################
>
> =item my $sql_type = $data->get_sql_type
> ***** end patch *****
>
> There are a couple other methods which do the same thing that may
> want to be
> removed/fixed as well: is_autopopulated, is_multiple
>
> Adrian


adrian at gossamer-threads

Mar 12, 2009, 6:50 PM

Post #4 of 8 (2212 views)
Permalink
Re: Patch to fix accessors in Bric::Biz::Element::Field [In reply to]

> Sorry, this fell through the cracks, though I know we did discuss it
> when I reviewed your patch. Please do feel free to change these methods,
> provided all tests continue to pass. Have you managed to get tests
> passing yet?

Yup, got a dev copy installed and got it going, thanks.

Got some failed tests though:

t/Bric/Test/Runner....NOK 9403/11815

# Failed test 'And it should be the right thing'
# at t/Bric/Util/ApacheReq/Test.pm line 23.
# (in Bric::Util::ApacheReq::Test->test_url)
# got: 'http://www.example.com:8080/'
# expected: 'http://www.example.com/'

Running Bricolage on a different port...


t/Bric/Test/Runner....NOK 10792/11815

# Failed test 'Check parse error payload'
# at t/Bric/Dist/Action/DTDValidate/DevTest.pm line 68.
# (in Bric::Dist::Action::DTDValidate::DevTest->test_do_it)
# '/foo/bad.html:14: parser error : Premature end of
data in tag html line 5
# '
# doesn't match '(?-xism:^/foo/bad\.html:11: (parser )?error ?:
Opening and ending tag mismatch: br line 10 and p)'
t/Bric/Test/Runner....NOK 10806/11815

# Failed test 'Check parse error payload'
# at t/Bric/Dist/Action/DTDValidate/DevTest.pm line 99.
# (in Bric::Dist::Action::DTDValidate::DevTest->test_do_it)
# '/foo/bad.html:14: parser error : Premature end of
data in tag html line 5
# '
# doesn't match '(?-xism:^/foo/bad\.html:11: (parser )?error ?:
Opening and ending tag mismatch: br line 10 and p)'

No clue what these are about.


t/Bric/Test/Runner....NOK 11470/11815

# Failed test 'test_href died (Unable to execute SQL statement:
DBD::Pg::st execute failed: ERROR: duplicate key value violates unique
constraint "udx_server_type__name_site" [for Statement "
# INSERT INTO server_type (id, name, description, site__id,
copyable, publish, preview, active, class__id)
# VALUES (NEXTVAL('seq_server_type'), ?, ?, ?, ?, ?, ?, ?,
(SELECT id FROM class WHERE LOWER(disp_name) = LOWER(?)))
# " with ParamValues: 1='Bogus1', 2='Bogus ServerType1',
3='100', 4='0', 5='1', 6='0', 7='1', 8='File System'] at
lib/Bric/Util/DBI.pm line 1136, <F> line 28.
#
# [lib/Bric/Util/DBI.pm:1136]
# [lib/Bric/Dist/ServerType.pm:2030]
# [t/Bric/Dist/ServerType/DevTest.pm:201]
# [/usr/share/perl5/Test/Class.pm:253]
# [/usr/share/perl5/Test/Class.pm:345]
# [t/Bric/Test/Runner.pm:101]
#
# [lib/Bric/Util/DBI.pm:1137]
# [lib/Bric/Dist/ServerType.pm:2030]
# [t/Bric/Dist/ServerType/DevTest.pm:201]
# [/usr/share/perl5/Test/Class.pm:253]
# [/usr/share/perl5/Test/Class.pm:345]
# [t/Bric/Test/Runner.pm:101])'
# at t/Bric/Test/Runner.pm line 101.
# (in Bric::Dist::ServerType::DevTest->test_href)

These didn't happen the first time around. There's a few more of this
variety.

I'm pretty sure these aren't linked to my accessor change, so I've
committed those changes.

Adrian


david at kineticode

Mar 12, 2009, 7:26 PM

Post #5 of 8 (2195 views)
Permalink
Re: Patch to fix accessors in Bric::Biz::Element::Field [In reply to]

On Mar 12, 2009, at 6:50 PM, Adrian Yee wrote:

>> Sorry, this fell through the cracks, though I know we did discuss
>> it when I reviewed your patch. Please do feel free to change these
>> methods, provided all tests continue to pass. Have you managed to
>> get tests passing yet?
>
> Yup, got a dev copy installed and got it going, thanks.
>
> Got some failed tests though:
>
> t/Bric/Test/Runner....NOK 9403/11815
> # Failed test 'And it should be the right thing'
> # at t/Bric/Util/ApacheReq/Test.pm line 23.
> # (in Bric::Util::ApacheReq::Test->test_url)
> # got: 'http://www.example.com:8080/'
> # expected: 'http://www.example.com/'
>
> Running Bricolage on a different port...

Don't do that. ;-P

> t/Bric/Test/Runner....NOK 10806/11815
> # Failed test 'Check parse error payload'
> # at t/Bric/Dist/Action/DTDValidate/DevTest.pm line 99.
> # (in Bric::Dist::Action::DTDValidate::DevTest->test_do_it)
> # '/foo/bad.html:14: parser error : Premature end
> of data in tag html line 5
> # '
> # doesn't match '(?-xism:^/foo/bad\.html:11: (parser )?error ?:
> Opening and ending tag mismatch: br line 10 and p)'
>
> No clue what these are about.

Make sure you're up-to-date with libxml2 and XML::LibXML.

> t/Bric/Test/Runner....NOK 11470/11815
> # Failed test 'test_href died (Unable to execute SQL statement:
> DBD::Pg::st execute failed: ERROR: duplicate key value violates
> unique constraint "udx_server_type__name_site" [for Statement "
> # INSERT INTO server_type (id, name, description,
> site__id, copyable, publish, preview, active, class__id)
> # VALUES
> (NEXTVAL('seq_server_type'), ?, ?, ?, ?, ?, ?, ?, (SELECT id FROM
> class WHERE LOWER(disp_name) = LOWER(?)))
> # " with ParamValues: 1='Bogus1', 2='Bogus ServerType1',
> 3='100', 4='0', 5='1', 6='0', 7='1', 8='File System'] at lib/Bric/
> Util/DBI.pm line 1136, <F> line 28.
> #
> # [lib/Bric/Util/DBI.pm:1136]
> # [lib/Bric/Dist/ServerType.pm:2030]
> # [t/Bric/Dist/ServerType/DevTest.pm:201]
> # [/usr/share/perl5/Test/Class.pm:253]
> # [/usr/share/perl5/Test/Class.pm:345]
> # [t/Bric/Test/Runner.pm:101]
> #
> # [lib/Bric/Util/DBI.pm:1137]
> # [lib/Bric/Dist/ServerType.pm:2030]
> # [t/Bric/Dist/ServerType/DevTest.pm:201]
> # [/usr/share/perl5/Test/Class.pm:253]
> # [/usr/share/perl5/Test/Class.pm:345]
> # [t/Bric/Test/Runner.pm:101])'
> # at t/Bric/Test/Runner.pm line 101.
> # (in Bric::Dist::ServerType::DevTest->test_href)
>
> These didn't happen the first time around. There's a few more of
> this variety.

Huh. That's not good. Sounds like there are some records that aren't
getting deleted from the database. Which is odd. Please look and see
if you have any records in the server_type table.

> I'm pretty sure these aren't linked to my accessor change, so I've
> committed those changes.

No, likely they're not.

Best,

David


adrian at gossamer-threads

Mar 12, 2009, 7:42 PM

Post #6 of 8 (2206 views)
Permalink
Re: Patch to fix accessors in Bric::Biz::Element::Field [In reply to]

>> t/Bric/Test/Runner....NOK 11470/11815
>> # Failed test 'test_href died (Unable to execute SQL statement:
>> DBD::Pg::st execute failed: ERROR: duplicate key value violates
>> unique constraint "udx_server_type__name_site" [for Statement "
>> # INSERT INTO server_type (id, name, description,
>> site__id, copyable, publish, preview, active, class__id)
>> # VALUES (NEXTVAL('seq_server_type'), ?, ?, ?, ?, ?, ?, ?,
>> (SELECT id FROM class WHERE LOWER(disp_name) = LOWER(?)))
>> # " with ParamValues: 1='Bogus1', 2='Bogus ServerType1',
>> 3='100', 4='0', 5='1', 6='0', 7='1', 8='File System'] at
>> lib/Bric/Util/DBI.pm line 1136, <F> line 28.
>> #
>> # [lib/Bric/Util/DBI.pm:1136]
>> # [lib/Bric/Dist/ServerType.pm:2030]
>> # [t/Bric/Dist/ServerType/DevTest.pm:201]
>> # [/usr/share/perl5/Test/Class.pm:253]
>> # [/usr/share/perl5/Test/Class.pm:345]
>> # [t/Bric/Test/Runner.pm:101]
>> #
>> # [lib/Bric/Util/DBI.pm:1137]
>> # [lib/Bric/Dist/ServerType.pm:2030]
>> # [t/Bric/Dist/ServerType/DevTest.pm:201]
>> # [/usr/share/perl5/Test/Class.pm:253]
>> # [/usr/share/perl5/Test/Class.pm:345]
>> # [t/Bric/Test/Runner.pm:101])'
>> # at t/Bric/Test/Runner.pm line 101.
>> # (in Bric::Dist::ServerType::DevTest->test_href)
>>
>> These didn't happen the first time around. There's a few more of this
>> variety.
>
> Huh. That's not good. Sounds like there are some records that aren't
> getting deleted from the database. Which is odd. Please look and see if
> you have any records in the server_type table.

Yup, the test entries were left around from the last run:

bricolage=> select * from server_type;
id | class__id | name | description | site__id | copyable |
------+-----------+--------+-------------------+----------+----------+
1223 | 12 | Bogus1 | Bogus ServerType1 | 100 | t |

1224 | 12 | Bogus2 | Bogus ServerType | 100 | f |
1225 | 12 | Bogus3 | Bogus ServerType3 | 100 | t |
1226 | 12 | Bogus4 | Bogus ServerType | 100 | f |
1227 | 12 | Bogus5 | Bogus ServerType5 | 100 | t |


lannings at who

Mar 13, 2009, 1:29 AM

Post #7 of 8 (2189 views)
Permalink
Re: Patch to fix accessors in Bric::Biz::Element::Field [In reply to]

On Thu, 12 Mar 2009, Adrian Yee wrote:
> Yup, the test entries were left around from the last run:

Have to clear out the DB before running make devtest.
In perldoc lib/Bric/Hacker.pod , section "TESTING":

You can get a clean database without reinstalling Bricolage by doing

sudo make db
sudo make db_grant


david at kineticode

Mar 13, 2009, 9:34 AM

Post #8 of 8 (2199 views)
Permalink
Re: Patch to fix accessors in Bric::Biz::Element::Field [In reply to]

On Mar 13, 2009, at 1:29 AM, Scott Lanning wrote:

> On Thu, 12 Mar 2009, Adrian Yee wrote:
>> Yup, the test entries were left around from the last run:
>
> Have to clear out the DB before running make devtest.
> In perldoc lib/Bric/Hacker.pod , section "TESTING":
>
> You can get a clean database without reinstalling Bricolage by doing

Yeah, but test data should not have been stuck in there…

David

Bricolage devel RSS feed   Index | Next | Previous | View Threaded
 
 


Interested in having your list archived? Contact Gossamer Threads
 
  Web Applications & Managed Hosting Powered by Gossamer Threads Inc.