Skip to content
This repository has been archived by the owner on Aug 5, 2019. It is now read-only.

Ddpb 2724 bypass case used exception #573

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

slizzio
Copy link
Contributor

@slizzio slizzio commented Jul 25, 2019

Purpose

To allow a lay deputy to register for a client when that client already exists under a professional deputyship.

Fixes DDPB-2724

Approach

After much deliberation I have made the client soft deletable. This is to avoid removing the client completely and any reports that might be associated. So the action of discharging a client is to make them soft deleted. This prevents the client appearing anywhere in client. However admin area may sttill want to access the client (and reports) from the old professional deputyship so for admin pages I have disabled the soft delete filtter and re-enabled it once the query has been run. This allows the client to appear twice in the event that a lay deputy has re-registered against the case number. To clearly identify which is which I have applied a discharged date where required on the admin pages.

Learning

Any tips and tricks, blog posts or tools which helped you. Plus anything notable you've discovered about DigiDeps

Checklist

  • I have performed a self-review of my own code
  • I have updated documentation (Confluence/GitHub wiki/tech debt doc) where relevant
  • I have added tests to prove my work, and they follow our best practices
  • I have successfully built my branch to a feature environment
  • New and existing unit tests pass locally with my changes (docker-compose run --rm api sh scripts/apiunittest.sh)
  • There are no Composer security issues (docker-compose run api php app/console security:check)
  • The product team have tested these changes

gregtyler
gregtyler previously approved these changes Jul 25, 2019
@@ -45,7 +45,7 @@ public function selfRegisterUser(SelfRegisterData $selfRegisterData)
$existingClient = $this->em->getRepository('AppBundle\Entity\Client')->findOneByCaseNumber(CasRec::normaliseCaseNumber($selfRegisterData->getCaseNumber()));

// ward off non-fee-paying codeps trying to self-register
if ($isMultiDeputyCase && $existingClient instanceof Client) {
if ($isMultiDeputyCase && ($existingClient instanceof Client) && $existingClient->hasDeputies()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The extra brackets here seem unnecessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Force of habit, I always put each condition in their own set of brackets in case anyone adds an or condition and a bug is introduced. Not necessary with all &&'s

@slizzio slizzio force-pushed the DDPB-2724-bypass-case-used-exception branch from a080324 to 9e2b615 Compare July 26, 2019 13:34
@gregtyler
Copy link
Contributor

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants