citusdata / django-multitenant

Python/Django support for distributed multi-tenant databases like Postgres+Citus
MIT License
708 stars 116 forks source link

Add m2m with no through_defaults fix #170

Closed grahamulator closed 1 year ago

grahamulator commented 1 year ago

This change addresses issue #168, (mentioned also in this comment).

In summary, attempting to add an m2m relationship when through_defaults are not passed as an argument causes the following error: TypeError: 'NoneType' object does not support item assignment

This change adds a line to django_multitenant/mixins.py to set through_defaults to an empty dict if no value is passed in, which prevents an assignment to None error.

grahamulator commented 1 year ago

@microsoft-github-policy-service agree company="Twingate"

grahamulator commented 1 year ago

Hi @gurkanindibay. Following up on our conversation here. I modified the Transaction model to better illustrate the scenario I described. The test I added is a duplicate of the one above it, but with no through_defaults passed in the add operation.

To reproduce the error scenario, simply comment out or remove the line I added to the mixins.py file and run the test.

Please feel free to modify this PR if you don't want to make changes to the models/migrations themselves, or let me know if I can change/improve in the PR. Thanks again!!

codecov[bot] commented 1 year ago

Codecov Report

Merging #170 (d35a9b4) into main (68270b1) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #170   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           43        44    +1     
  Lines         1180      1193   +13     
=========================================
+ Hits          1180      1193   +13     
Impacted Files Coverage Δ
...nt/tests/migrations/0031_alter_transaction_date.py 100.00% <100.00%> (ø)
django_multitenant/tests/models.py 100.00% <100.00%> (ø)
django_multitenant/tests/test_models.py 100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more