bcgov / entity

ServiceBC Registry Team working on Legal Entities
Apache License 2.0
23 stars 59 forks source link

Invalid Timestamp for some Name Requests #22243

Open Mihai-QuickSilverDev opened 4 months ago

Mihai-QuickSilverDev commented 4 months ago

To Do List:

EPortman commented 3 months ago

Currently the table in Postgres has a datetime_precision of 6 (this allows for both milliseconds and microseconds).

image.png

An example of one of the entries with milliseconds is here: 2024-07-18 15:53:51.717227+00. What we want is to not have anything after the period (the precision should be 0).

To do this the following query can be made against the database.

ALTER TABLE requests
ALTER COLUMN expiration_date TYPE TIMESTAMP(0) WITH TIME ZONE;

OR it can be done with alembic

def upgrade():
    # Alter the column to remove milliseconds
    op.alter_column(
        'requests',
        'expiration_date',
        type_=sa.TIMESTAMP(timezone=True, precision=0),
        existing_type=sa.TIMESTAMP(timezone=True),
        existing_nullable=True
    )

This should probably be done for every date field for consistency.

EPortman commented 3 months ago

@ozamani9gh doing the above fix would make it so it is impossible for a date record to have milliseconds no matter what query is being run against it.

Would you want me to implement this or create a new ticket for it?

EPortman commented 3 months ago

As far as why this is happening - because no constraint was made on the table.

EPortman commented 3 months ago

Currently in prod > 50k NRs have expiration dates that are correctly formatted.

However ~ 9k NRs are improperly formatted with these extra digits of precision.

image.png

This data can be fixed by running this SQL:

UPDATE requests
SET expiration_date = date_trunc('second', expiration_date)
WHERE expiration_date IS NOT NULL;
ozamani9gh commented 3 months ago

@EPortman i would chat with Eve tommorow morning. we are now setting the expiration_date in modernization and not going to use the Sync job to bring back the expiration_date. i would say ask Eve what precision we should set it to so LEAR does not break.

we can discuss tommorow with eve if you would like.

EPortman commented 3 months ago

@ozamani9gh sounds good. I thought it would need to be thought over before making database changes which is why I held off on doing anything.

Lets have a chat tomorrow and figure out if this problem can be fixed by simply changing the precision date at the database level.

EPortman commented 3 months ago

I spoke with Entities team and they have fixed this on their side and have no need for us to make modifications in NameX.
https://github.com/bcgov/lear/pull/2798/files

Since the expiration date is stable and not currently causing any problems, we decided this does not need to be fixed - at least not right now.

Moving this ticket to done.

Attached ticket from Entities Board with more information https://app.zenhub.com/workspaces/entities---david-65af11a89e89f5043c2911f6/issues/gh/bcgov/entity/22094

EPortman commented 3 months ago

The issue was due to the release of NameX on June 19th that causes expiration dates to have milliseconds when they go through a PUT request.

Attached PR fixing this issue: https://github.com/bcgov/namex/pull/1564

EPortman commented 3 months ago

For Quality Assurance:

This one is another tricky one to do QA for. Ideally one would have access to the Names Database through PGAdmin to see how the Expiration dates are getting formatted.

However, the expiration dates were getting improperly formatted when the save edits button was clicked.

image.png

In the picture above there is an NR with number NR 3158401 with expiry date 2024-10-17. This is what the data looks like in the database (correctly formatted with no milliseconds).

image.png

I then update this NRs expiry date to a couple of days after and click save edits.

image.png

When I look in the database now the data is updated but still in the correct format without milliseconds (the day after what is specified because it expires at the end of the day).

image.png
Mihai-QuickSilverDev commented 3 months ago
Mihai-QuickSilverDev commented 3 months ago
gunanagar commented 14 hours ago

Tested, looks good