Feature/no role

[domdinicola]

No description provided.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.18%. Comparing base (9fbf3a0) to head (02cb227).

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4906      +/-   ##
===========================================
- Coverage    86.18%   86.18%   -0.01%     
===========================================
  Files          657      658       +1     
  Lines        36159    36163       +4     
===========================================
+ Hits         31165    31167       +2     
- Misses        4994     4996       +2     

Flag	Coverage Δ
e2e	86.18% <100.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[pavlo-mk]

@domdinicola how about data migration IndividualRoleInHousehold with ROLE_NO_ROLE do we have in prod/trn/stg any records?

[pavlo-mk]

and coverage

[pkujawa]

Same as in this PR #4894 - we need to make sure that this option is still displayed in the dropdown. We need the None option in the dropdown to be able to create a grievance ticket for changing a role for individual primary/alternate -> No role. (So the option to remove the role from the individual) Either by adding the None option to the role choices for frontend or adding it on frontend already. And please make sure that this kind of ticket is still creatable and removes the role correctly.

[pkujawa]

You need to retrieve the .role from the object.

        role = self.households_and_roles.first()
        return role.role if role else None

The same as before just different default if individual does not have any IndividualRoleInHousehold

[pkujawa]

I think just return self.households_and_roles.count() since now if the IndividualRoleInHousehold obj exists, it will only have one of these two options.

[pkujawa]

Also in
def handle_role we can change
if role in (ROLE_PRIMARY, ROLE_ALTERNATE) and household: -> if role and household:
and change typing since we probably will take None now as the change for no role - this flow needs to be adjusted with frontend.

[pavlo-mk]

LGTM
@pkujawa could you check this one please?

[pkujawa]

I don't see the previous comments addressed.

1. This PR would make it impossible to remove role for individual via Grievance Ticket (PR #4968 needs to be merged first and then make sure it is possible to remove existing role as there will be no option NO_ROLE)
2. Also it will be impossible to create a Targeting with filter for role=None as it is not within choices now.
3. I can see there are a lot of usages of ROLE_NO_ROLE still. Is it planned to do only part now?
   Like for example

    def count_all_roles(self) -> int:
        return self.households_and_roles.exclude(role=ROLE_NO_ROLE).count()

There are tests assigning ROLE_NO_ROLE for IndividualRoleInHousehold but it's not within choices anymore.

We return ROLE_NO_ROLE as a role if it's None. It's something we want?

    def role(self) -> str:
        role = self.households_and_roles.first()
        return role.role if role is not None else ROLE_NO_ROLE

4. My previous comment:
   def handle_role we can change
   if role in (ROLE_PRIMARY, ROLE_ALTERNATE) and household: -> if role and household:
   and change typing since we will probably take None now as the change for no role.

I can see a lot of changes were removed from this PR that were cleaning the usages of ROLE_NO_ROLE. Do we plan to remove this value completely or we want to keep it in some places?

[pavlo-mk]

one question about grievance... do we cover every think there if we haven't yet None?

[pkujawa]

one question about grievance... do we cover every think there if we haven't yet None?

We need to make sure that together with this PR we are able to remove existing roles without the option NO_ROLE. handle_role will then receive None for this case and will remove the existing role. But this needs to be handled in the flow.