updating destroy/destroy_all documenatation [ci skip] #13086

Open
wants to merge 1 commit into
from

Projects

None yet

2 participants

@kuldeepaggarwal

Updated the documentation. #13085, #17159

@rafaelfranca
Ruby on Rails member

Have you confirmed if this is the desirable behavior?

@kuldeepaggarwal

Tested on my local machine.

@rafaelfranca
Ruby on Rails member

But this doesn't means it is not a bug.

@kuldeepaggarwal

And it is working as expected when destroy is called and is not ignoring dependent option.

@rafaelfranca
Ruby on Rails member

I still don't have sure that it should not be ignoring dependent option, maybe it should.

@rafaelfranca
Ruby on Rails member

@kuldeepaggarwal what we can do now is check if we have tests for this behavior in our test suite, if so it is really intentional. Could you take a look and link in one comment here?

@kuldeepaggarwal

@rafaelfranca Yeah Sure, I am looking the test cases and will update you asap.

@kuldeepaggarwal

@rafaelfranca I have not found any test case for the original issue created #13085, neither found any test case for destroy_all with three levels of dependency having dependent options, restrict_with_error, restrict_with_exception.. 😞

@rafaelfranca
Ruby on Rails member

I see. I think we only need to check if there are tests to asset that destroy_all take in consideration :dependent option. We don't have tests to this?

@rafaelfranca
Ruby on Rails member

Unfortunately that test is not sufficient to tell if this is desirable behavior or not. We will have to investigate the git history to find when that documentation was added and check if it really behaves that way.

@kuldeepaggarwal

commit messages when docs were updated, related to delete_all, destroy and destroy_all:

There we go, documentation has been updated for delete_all:
03402d2
f9a718e

and updated for destroy and destroy_all:
d831734

@rafaelfranca
Ruby on Rails member

Great, now we need to go back to d831734 and check if in that commit destroy_all ignores the :dependent option.

@kuldeepaggarwal

@rafaelfranca May be at that time destroy_all ignores the :dependent: option but now it doesn't, so I think we should correct it. Isn't it?

@kuldeepaggarwal

@rafaelfranca any updates on this?

@kuldeepaggarwal

@rafaelfranca any suggestions?

@kuldeepaggarwal

@rafaelfranca I think we should close this ?

@rafaelfranca rafaelfranca locked and limited conversation to collaborators Jun 13, 2014
@rafaelfranca
Ruby on Rails member

I'll take a look when I have time to investigate what is the expected behavior.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.