Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Import licence document for a licence #1392

Merged
merged 20 commits into from
Oct 16, 2024
Merged

Conversation

jonathangoulding
Copy link
Collaborator

https://eaflood.atlassian.net/browse/WATER-4669

We need to replace the import service logic to import a licence from NALD.

WRLS has the concept of a licence document. NALD does not.

As part of the import process this licence document is created from the NALD licence. The start and end date use the licence and licence versions to calculate the earliest date for both.

This change will replicate this logic.

We will insert this imported data in the relevant public views.

https://eaflood.atlassian.net/browse/WATER-4669

We need to replace the import service logic to import a licence from NALD.

WRLS has the concept of a licence document. NALD does not.

As part of the import process this licence document is created from the NALD licence. The start and end date use the licence and licence versions to calculate the earliest date for both.

This change will replicate this logic.

We will insert this imported data in the relevant public views.
@jonathangoulding jonathangoulding added the enhancement New feature or request label Oct 8, 2024
@jonathangoulding jonathangoulding self-assigned this Oct 8, 2024
@jonathangoulding jonathangoulding marked this pull request as ready for review October 15, 2024 13:35
Copy link
Member

@Cruikshanks Cruikshanks left a comment

Choose a reason for hiding this comment

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

A couple of copy and paste typos.

The one change I'd make is not to overwrite the deletedAt date when we upsert. But that also raises the question of how does the import currently determine deletedAt?

Once we know that we can make a decision as to whether it should be included in this change or added separately.

'endDate',
'startDate',
'updatedAt',
'deletedAt'
Copy link
Member

Choose a reason for hiding this comment

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

I'd have to dig into how deletedAt is determined (because I think that is set during the import as well).

But we shouldn't be overwriting this if it is already set.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The import sets this to null on the insert in the original query.

I took this as something has changed in NALD and now the data is no longer 'deleted' ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I see what you mean now.

Copy link
Collaborator Author

@jonathangoulding jonathangoulding Oct 16, 2024

Choose a reason for hiding this comment

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

-- auto-generated definition
create table documents
(
    document_id   uuid      default gen_random_uuid()                        not null
        primary key,
    regime        varchar   default 'water'::character varying               not null,
    document_type varchar   default 'abstraction_licence'::character varying not null,
    document_ref  varchar                                                    not null,
    start_date    date                                                       not null,
    end_date      date,
    external_id   varchar,
    date_created  timestamp default now()                                    not null,
    date_updated  timestamp default now()                                    not null,
    is_test       boolean   default false                                    not null,
    date_deleted  timestamp
);

I can remove 'deletedAt' from the upsert.

It will default to null if not set and not overide the existing logic.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Change made and pushed to remove the deletedAt logic

Copy link
Member

@Cruikshanks Cruikshanks left a comment

Choose a reason for hiding this comment

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

Answered my own question. deletedAt gets set by another job: src/modules/licence-import/jobs/delete-removed-documents.js.

It wouldn't apply in this case as it happens when a user deletes a licence from NALD (which they are not supposed to do but is possible).

So, not applicable to this change or our current import process.

@jonathangoulding jonathangoulding merged commit dbeb9c0 into main Oct 16, 2024
6 checks passed
@jonathangoulding jonathangoulding deleted the import-licence-document branch October 16, 2024 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants