Little resignation improvements - deleting end_dates

[kerstenkenan]

Hi @Theophile-Madet, while checking out further your implementations for the resignation issue, I fall over (what seems to me) a little bug, after I tried to cancel and remove a resigned member from the list (with 3 years cash back) here on my local. With the former filtering in the delete_end_dates function checking whether the cancellation_date equals the end_date, so the end_dates of peoples with shares ending 3 years in the future don't get set to None. Hope this PR makes sense to you?!

[Theophile-Madet]

Good find on the bug,
I think we should fix it a bit differently, I left comments.

[Theophile-Madet]

I think we want to be more specific about which shares end_date we remove. For the cases where the cancellation was instant, we can remove the end date of all shares that are after the cancellation date, as you did.

For ResignationType.BUY_BACK, we should only remove the end dates of shares that are at or after the pay out day.

When updating several shares, you can do it this way:

Suggested change

	for share_ownership in resignation.share_owner.share_ownerships.all():
	if share_ownership.end_date >= resignation.cancellation_date:
	share_ownership.end_date = None
	share_ownership.save()
	resignation.share_owner.share_ownerships.filter(
	end_date__gte=resignation.cancellation_date
	).update(end_date=None)

[Theophile-Madet]

I think this should be self.assertEqual instead of self.assertTrue

It's a good idea to update the test, but this is not the correct test to update: this one only checks that on_resignation_deleted gets called. Since on_resignation_deleted is replaced with a mock function, it is not actually executed, inside the test the dates don't actually get updated.

I think we want a new test, similar to test_deleteEndDates_default_sharesEndDateSetToNone, that instead does test_deleteEndDates_sharesEndOnPayOutDay_sharesEndDateSetToNone

[kerstenkenan]

I think this is done by test_deleteEndDates_someSharesEndedBeforeCancellation_thoseSharesKeepTheirEndDate in test_service.py in the last line. Don't you think?

[Theophile-Madet]

Not quite: in test_deleteEndDates_someSharesEndedBeforeCancellation_thoseSharesKeepTheirEndDate, we check that shares that ended before the cancellation are not affected.

In case of share buy back, we want to check that shares that end after the pay out day have their end-date set to none, and that shares that end before the pay out day are not affected.