Skip to content

Member creation 2 of 3 ‐ Creating an Member

Jump to bottom
Théophile MADET edited this page Jun 28, 2024 · 11 revisions

When an applicant signs their membership agreement, they are officially a member of the cooperative. Members, current past or future, are represented by the ShareOwner model. The CreateShareOwnerFromDraftUserView handles the transition from DraftUser to ShareOwner and has a few interesting features.

Summary of what the view does

CreateShareOwnerFromDraftUserView goes through the following steps:

  • Checks that the member can be created: draft_user.can_create_user()
  • Displays a message explaining why it can't be created if necessary: messages.error(...)
  • Creates the member objects and the appropriate number of share objects: create_share_owner_and_shares_from_draft_user()
  • Links the DraftUser to the new ShareOwner, thus marking the DraftUser as done: draft_user.share_owner = share_owner
  • Creates a NewMembershipsForAccountingRecap object. Once a week a report of the new memberships is sent to the accounting team (see tapir/coop/management/commands/send_accounting_recap.py)
  • Sends an email to the member, the content depending on wether the new member is investing or not:
email = (
    MembershipConfirmationForInvestingMemberEmail
    if share_owner.is_investing
    else MembershipConfirmationForActiveMemberEmail
)(share_owner=share_owner)
email.send_to_share_owner(actor=request.user, recipient=share_owner)

Never trust user input

The button to create a member from an applicant is in the DraftUser detail template. This button is disabled if the member cannot be created yet, typically if the membership agreement has not been signed yet, so for normal, innocent users it is not possible to access CreateShareOwnerFromDraftUserView if the member cannot be created. However, it is still very important to check that the member can be created in the view: the button can be enabled with the browser dev tools, another user could have created a member at the same time from a different machine, a malicious user could try to manipulate our data...

Therefore, while it is useful to have our UI show which actions are possible and which are not for usability reasons, it is not sufficient to ensure data integrity and security. Always check that the actions can be performed, even if you are sure that the UI only allows valid cases.

Messages

Messages are a nice way to confirm that an action has been performed or explain why it failed. They look like this and are sent by calling django.contrib.messages.error("...") or info/warning/...("...")

The base template is responsible for displaying those messages, see bootstrap_messages.

Atomic transactions

Creating a DraftUser consists of only one database operation. That means that if that operation fails, whatever the reason, the object won't be created. That's the behaviour we want.

Creating a ShareOwner consists of several operations, as we can see in tapir.coop.views.draftuser.CreateShareOwnerFromDraftUserView.get and tapir.coop.views.draftuser.create_share_owner_and_shares_from_draft_user. This is dangerous because if an error happens in between two operations, we could end up with invalid data in our database. To avoid this risk, we use with transaction.atomic(): if any error happens inside the with, any change that has been applied since the beginning of the transaction will be rolled back.

You'll probably want to wrap at least part of your data-manipulating views with with transaction.atomic(): or @transaction.atomic.

Avoid deleting data

You may have noticed that after creating the ShareOwner, we don't delete the DraftUser but instead fill the DraftUser.share_owner field. Since all the informations are copied from DraftUser to ShareOwner, we should not need the DraftUser and could delete it. However, experience has shown that we often want to look at the history of changes, for example to fix an error or to explain why the current state is as it is.

That's why, as much as possible, we should avoid deleting database objects and instead mark them as deleted with a "deleted" or "end_date" field. This principle is not well applied yet in Tapir at the time of writing.

User information fields are duplicated in several models

When we create the ShareOwner (and later when we'll create the TapirUser), we copy the personal information from the existing object to the new one. See copy_user_info in tapir.coop.views.draftuser.create_share_owner_and_shares_from_draft_user. This is not ideal because it makes it unclear where the information should be fetched from and could lead to inconsistent data between to corresponding objects.

To mitigate this risk:

  • We empty the fields from the old object when copying to a new one. This makes errors more visible when fetching data from an old object, compared to showing potentially outdated data.
  • We call DraftUser/ShareOwner/TapirUser.get_info() before accessing any personal information, which gives us the correct object to fetch from.
Clone this wiki locally