Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix compatibility with 0.34 and add test code #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bokutin
Copy link

@bokutin bokutin commented Jul 10, 2020

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
Copy link
Author

bokutin commented Jul 10, 2020

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant