Cas2bail/main #2641

garethCAS2 · 2024-12-04T15:25:25Z

No description provided.

davidatkinsuk · 2024-12-04T16:04:00Z

Looks like this snuck in - src/main/kotlin/uk/gov/justice/digital/hmpps/approvedpremisesapi/controller/cas2bail/.keep. Could you please remove and add .keep to the .gitignore file

davidatkinsuk

Hiya. I've done a first pass on this. You may want to ignore the comments where i'm suggesting code copied from CAS2 my want improving as i'm not sure what the agreed approach is here (e.g. remove use of deprecated functions)

davidatkinsuk · 2024-12-04T16:04:47Z

src/main/resources/application-local.yml

@@ -67,7 +71,8 @@ logging:
    org.hibernate.SQL: DEBUG
    # Uncomment the two entries below to see SQL binding
    #org.hibernate.orm.jdbc.bind: TRACE
-    #org.hibernate.type.descriptor.sql.BasicBinder: TRACE
+    org.hibernate.type.descriptor.sql.BasicBinder: TRACE
+    org.hibernate.type.descriptor.sql: tract


could you please revert this as it makes the logs quite noisy by default

davidatkinsuk · 2024-12-04T16:06:03Z

.../db/migration/all/20241128163133__application_controller_and_linked_services_in_cas2bail.sql

+(
+    id                          UUID NOT NULL,
+    application_id              UUID,
+    created_at                  TIMESTAMP WITHOUT TIME ZONE,


we use timestamptz everywhere so should do the same for all entries here

It's also not recommended by postgres - https://wiki.postgresql.org/wiki/Don't_Do_This#Don.27t_use_timestamp_.28without_time_zone.29

davidatkinsuk · 2024-12-04T16:06:38Z

src/main/resources/static/_shared.yml

@@ -1943,6 +1943,49 @@ components:
        - personName
        - crn
        - nomsNumber
+    Cas2BailApplicationSummary:


i think this should go in cas2bail-schemas.yml?

Agreed done

davidatkinsuk · 2024-12-04T16:06:47Z

src/main/resources/static/cas2bail-schemas.yml

+
+# GARETH above here
+
+# TOBY below here


davidatkinsuk · 2024-12-04T16:09:26Z

...stice/digital/hmpps/approvedpremisesapi/controller/cas2bail/Cas2BailApplicationController.kt

+@Service(
+  "uk.gov.justice.digital.hmpps.approvedpremisesapi.controller.cas2bail" +
+    ".Cas2BailApplicationsController",
+)


because the class name is unique within the project, you shouldn't need to specify a value in the @service annotation

Agreed done.

davidatkinsuk · 2024-12-04T16:30:08Z

...justice/digital/hmpps/approvedpremisesapi/service/cas2bail/Cas2BailApplicationNoteService.kt

+    if (application.createdByUser.email != null) {
+      emailNotificationService.sendCas2Email(
+        recipientEmailAddress = application.createdByUser.email!!,
+        templateId = notifyConfig.templates.cas2NoteAddedForReferrer,


are the existing cas2 email templates (defined in notify) going to be ok for cas2 bail?

davidatkinsuk · 2024-12-04T16:34:20Z

...gov/justice/digital/hmpps/approvedpremisesapi/service/cas2bail/Cas2BailApplicationService.kt

+      return sanitisedData
+    }
+    return null
+  }


code like this feels suitable for reuse instead of copy-pasting?

davidatkinsuk · 2024-12-04T16:46:51Z

...ce/digital/hmpps/approvedpremisesapi/transformer/cas2bail/Cas2BailApplicationsTransformer.kt

+
+
+  fun transformJpaToApi(jpa: Cas2BailApplicationEntity, personInfo: PersonInfoResult): Cas2Application {
+    return Cas2Application(


just to confirm - we're reusing the Cas2Application API type for Cas2Bail?

Currently yes but this will likely change

davidatkinsuk · 2024-12-04T16:48:39Z

.../db/migration/all/20241128163133__application_controller_and_linked_services_in_cas2bail.sql

+    id                          UUID NOT NULL,
+    application_id              UUID,
+    created_at                  TIMESTAMP WITHOUT TIME ZONE,
+    body                        VARCHAR(255),


we should use text

it's not recommended to use VARCHAR - https://wiki.postgresql.org/wiki/Don't_Do_This#Don.27t_use_varchar.28n.29_by_default

davidatkinsuk · 2024-12-04T16:49:25Z

.../db/migration/all/20241128163133__application_controller_and_linked_services_in_cas2bail.sql

+    id                          UUID NOT NULL,
+    application_id              UUID,
+    created_at                  TIMESTAMP WITHOUT TIME ZONE,
+    body                        VARCHAR(255),


can you please revise this file to match the Cas2 database types on equivalent tables? If you think they're wrong give me a shout and we can discuss

davidatkinsuk · 2024-12-04T17:03:53Z

...ov/justice/digital/hmpps/approvedpremisesapi/service/cas2bail/Cas2BailStatusUpdateService.kt

+    val newStatus = statusUpdate.status()
+    val assessor = statusUpdate.assessor
+
+    domainEventService.saveCas2ApplicationStatusUpdatedDomainEvent(


sanity check - are we ok with reusing existing CAS2 domain events for CAS2 bail applications?

For now we are using them but we expect them to possibly change.

williamfalconeruk

I don't think this the approach we should be taking, copying and pasting wholesale with tweaks.

As far as i understood it the aim was to reuse the existing back end endpoints and DB for as much of the CAS2 bail with differentiators added as needed.

i don't understand the rationale behind the change in approach, other than to de risk CAS 2 HDC now - but is most likely a false economy.

I think by doing this we are incurring a large amount of technical debt which the internal team will have to pay down and dual maintain. I am keen for us to avoid that. I am not recommending we merge this at this stage until we've worked out the best way forward.

williamfalconeruk · 2024-12-05T10:39:02Z

...stice/digital/hmpps/approvedpremisesapi/controller/cas2bail/Cas2BailApplicationController.kt

+        )
+
+    ) {
+      is AuthorisableActionResult.NotFound -> null


my position is we shouldn't use deprecated code, and use the latest pattern we have. please can we do this?

Toby Batch and others added 30 commits November 28, 2024 10:36

feat: added open api scaffold for cas2 to cas2bail routes and delegates

c8de685

first round of cas2bail application

45474a5

feat: application controller and linked services in cas2bail

0402efd

feat: added migrations

4291eef

started working through the tests

387da56

initial unit tests working - for cas2bail application

6166492

unit tests for Cas2BailAssessments and Cas2UserAccessService

0b40d88

wip tests and CasResult

f16a186

More tests

162c90d

WIP around tests

3a2baaa

feat: adds the view for live summary

3ddd76b

fix: merged new bail migrations

a6964b1

chore: more test debug

c8d6247

chore: added Cas2BailAssessmentEntityFactory

e7c5f01

chore: more test changes and some renaming

dc1abef

chore: debug code for why the JPA isn't writing to some objects

86ab963

feat: Cas2BailApplication tests running

4c784be

feat: Cas2BailApplicationAbandonTest added

adacbf8

feat: Cas2BailAssessmentTest

790d541

feat: Cas2BailAssessmentTest renamed the tests

c11e561

feat: Cas2BailPeopleController and tests

c1afc7b

chore: wip

2048331

first round of cas2bail application

134b3a0

feat: added migrations

5f74c57

started working through the tests

947158b

feat: adds the view for live summary

b1001e2

chore: more test debug

12d980c

feat: Cas2BailApplication tests running

06d1429

feat: Cas2BailApplicationAbandonTest added

c5a64b0

feat: Cas2BailAssessmentTest

459044e

garethCAS2 requested a review from davidatkinsuk December 4, 2024 15:25

feat: cas2Reports and tests and cas2StatusUpdate and tests

4390bcf

davidatkinsuk requested changes Dec 4, 2024

View reviewed changes

davidatkinsuk reviewed Dec 4, 2024

View reviewed changes

Toby Batch and others added 5 commits December 5, 2024 09:16

fix: removed .keep

d0691ea

Merge branch 'cas2bail/main' into cas2bail/submissions

dd232ae

chore: exploding more imports

cfc809a

fixes: for linter and detekt

50be8e3

fix: wildcard imports

b6b1b46

williamfalconeruk requested changes Dec 5, 2024

View reviewed changes

garethCAS2 added 4 commits December 5, 2024 11:00

Merge branch 'cas2bail/main' into cas2bail/statusUpdate

ee33a8e

fixes for linter and detekt

64704fc

fix: added import to Cas2BailApplicationEntity

2c6b359

Added comment where we think code will start to change after Xmas

13e4eed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Cas2bail/main #2641

Cas2bail/main #2641

garethCAS2 commented Dec 4, 2024

davidatkinsuk commented Dec 4, 2024

davidatkinsuk left a comment

davidatkinsuk Dec 4, 2024

garethCAS2 Dec 5, 2024

davidatkinsuk Dec 4, 2024

garethCAS2 Dec 5, 2024

davidatkinsuk Dec 4, 2024

garethCAS2 Dec 5, 2024

davidatkinsuk Dec 4, 2024

davidatkinsuk Dec 4, 2024

garethCAS2 Dec 5, 2024

davidatkinsuk Dec 4, 2024

davidatkinsuk Dec 4, 2024

davidatkinsuk Dec 4, 2024

garethCAS2 Dec 5, 2024

davidatkinsuk Dec 4, 2024

garethCAS2 Dec 5, 2024

davidatkinsuk Dec 4, 2024

davidatkinsuk Dec 4, 2024

garethCAS2 Dec 5, 2024

williamfalconeruk left a comment

williamfalconeruk Dec 5, 2024



		fun transformJpaToApi(jpa: Cas2BailApplicationEntity, personInfo: PersonInfoResult): Cas2Application {
		return Cas2Application(

Cas2bail/main #2641

Are you sure you want to change the base?

Cas2bail/main #2641

Conversation

garethCAS2 commented Dec 4, 2024

davidatkinsuk commented Dec 4, 2024

davidatkinsuk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

williamfalconeruk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment