Feat auto indexing

[calitidexvii]

PR for #96 feature request "Add post_save and post_delete signals"

[calitidexvii]

The key thing to note is that I set auto indexing to be automatic (the signals are added by default). Unless you decide to disable it by default, existing projects that are upgrading would need to note this change in order to prevent duplicate indexing operations on their instance.

The PR includes docs for two features (auto-indexing and a decorator to disable auto-indexing) as well as the new setting to control whether auto-indexing is enabled for the project.

Tests are included to verify connecting/disconnecting the signals as well as testing the new setting.

In order to not mess up the existing tests, I added the disable_auto_indexing decorator to all of the tests that called save(). This also ensures that you're testing the indexing functions directly without the signals getting in the way.

[CorrosiveKid]

@calitidexvii Will you have some time to figure out why the coverage step is failing please?

[calitidexvii]

@CorrosiveKid The tests are completing ok. The issue is with the CODECOV_TOKEN secret in test.yml when generating the coverage report. According to the Codecov docs, a token isn't required for public repos. So, I think you should be to remove -t ${{ secrets.CODECOV_TOKEN }} from line 32. The -t switch is what's breaking because there's no arg being passed.

[codecov]

Codecov Report

Merging #97 into master will decrease coverage by 1.85%.
The diff coverage is 90.90%.

@@             Coverage Diff             @@
##            master      #97      +/-   ##
===========================================
- Coverage   100.00%   98.14%   -1.86%     
===========================================
  Files            7        9       +2     
  Lines          138      162      +24     
  Branches        19       18       -1     
===========================================
+ Hits           138      159      +21     
- Misses           0        2       +2     
- Partials         0        1       +1     

Impacted Files	Coverage Δ
django_elastic_appsearch/orm.py	95.31% <62.50%> (-4.69%)	⬇️
django_elastic_appsearch/apps.py	100.00% <100.00%> (ø)
django_elastic_appsearch/decorators.py	100.00% <100.00%> (ø)
django_elastic_appsearch/signals.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d99235...bb3e461. Read the comment docs.

[calitidexvii]

@CorrosiveKid I can add tests for those two lines missing from coverage. Do you need any other changes?

[CorrosiveKid]

@CorrosiveKid I can add tests for those two lines missing from coverage. Do you need any other changes?

@calitidexvii Thanks for that. Another change I think might be good is to make the auto indexing disabled by default, and change the decorator to enable_auto_indexing. This will make sure any applications that are currently using this library won't be broken because of this change.

[calitidexvii]

@CorrosiveKid I set auto-indexing default to False and removed the disable_auto_indexing decorator from the tests since it's no longer needed there.

I left the decorator as disable_auto_indexing rather than enable because the purpose of the decorator is the ability to contextually disable auto-indexing when it's enabled.

I ran into a problem writing tests for this because override_settings doesn't reinitialize app configs and attaching the signals are dependent on the app config. I asked the question here in case you have any suggestions. To test the decorator for now, I used the workaround mentioned at the end of the question, but I'd rather not write code just to suit tests. And we're still missing coverage for attaching the signals in __init_subclass__ in AppSearchModel because of initialization order (ie, the workaround I used for the decorator doesn't work here). An option would be to set APPSEARCH_AUTOINDEXING_ENABLED to True in the test settings, but I'm trying to avoid that if possible.

[CorrosiveKid]

@calitidexvii

I left the decorator as disable_auto_indexing rather than enable because the purpose of the decorator is the ability to contextually disable auto-indexing when it's enabled.

That makes sense, thanks for that.

I think I ran into the same problem sometime back with override_settings not really working because of app config. I'll think about this a bit as well, see if I have any ideas to get around this issue.