gshank / dbix-class-resultset-recursiveupdate

DBIx::Class::ResultSet::RecursiveUpdate
4 stars 9 forks source link

fix compatibility with 0.34 and add test code #19

Open bokutin opened 4 years ago

bokutin commented 4 years ago

Hello.

1. Incompatibility

I recently upgraded to 0.40 and my existing code is no longer working.

It's a form containing a simple many_to_many using HTML::FormHandler::Model::DBIC.

I wrote test code in t/99_m2m_form_compat.t that reproduces the problem.

% curl -so - https://cpan.metacpan.org/modules/by-module/DBIx/DBIx-Class-ResultSet-RecursiveUpdate-0.34.tar.gz | tar xzf - % curl -so - https://cpan.metacpan.org/modules/by-module/DBIx/DBIx-Class-ResultSet-RecursiveUpdate-0.40.tar.gz | tar xzf - % curl -so - https://cpan.metacpan.org/modules/by-module/DBIx/DBIx-Class-ResultSet-RecursiveUpdate-0.41.tar.gz | tar xzf -

% prove -I DBIx-Class-ResultSet-RecursiveUpdate-0.34/lib t/99_m2m_form_compat.t pass % prove -I DBIx-Class-ResultSet-RecursiveUpdate-0.40/lib t/99_m2m_form_compat.t fail % prove -I DBIx-Class-ResultSet-RecursiveUpdate-0.41/lib t/99_m2m_form_compat.t fail % prove -I lib t/99_m2m_form_compat.t (This pull request has been applied) pass

2. Cause of the problem

For set_$rel it works fine. For IntrospectableM2M it does not work.

For the latter, the problem is the implicit assumption that the relation name and pk are the same. Not the same naming also let's assume.

3. differences between set_$rel and IntrospectableM2M

There are cases where it works with set_$rel but not IntrospectableM2M.

It's in the current 0.41 and it's also in 0.34 and earlier.

I added test code to t/99_m2m_form.t and t/99_m2m_pass.t so that we can test it in the common case.

I think this test needs to pass.


My changes may not be good code to be merged as-is, but I hope they reveal the problem.

Since 0.40, I think the reduction of aggressive select queries is very nice!

Thanks.

bokutin commented 4 years ago

It turns out that this pull request fails the HTML-FormHandler-Model-DBIC-0.29 test. I'm sorry. I will check.