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

WIP MCP-1496 flyway scripts #165

Closed
wants to merge 4 commits into from
Closed

WIP MCP-1496 flyway scripts #165

wants to merge 4 commits into from

Conversation

dimitri-amida
Copy link
Contributor

@dimitri-amida dimitri-amida commented Jul 21, 2022

Feature Name

Description

What was the problem?

How does this fix it?

Jira Tickets

How to test this PR

  • Step 1
  • Step 2

Reminders

  • Review Pull-Requests guidelines
  • If PR is significant, update Current Software State and
    Roadmap wiki pages
  • If changing software behavior, post a link to the PR in Slack channel #benefits-virtual-regional-office

@dimitri-amida dimitri-amida changed the title MCP-1496 flyway scripts WIP MCP-1496 flyway scripts Jul 21, 2022
@ianwolson ianwolson marked this pull request as ready for review July 21, 2022 20:56
Copy link
Contributor Author

@dimitri-amida dimitri-amida left a comment

Choose a reason for hiding this comment

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

@ianwolson I have added some preliminary comments.
The main ideas is that these tables should match the entity definitions under gov.va.vro.persistence.model,

Tomorrow, let's create an integration test to make sure that this happens.

Also, we need to delete this PR so you can start one. Since I started it, I cannot properly review it. Sorry about that

PRIMARY KEY(veteran_uuid),
icn VARCHAR(128) NOT NULL UNIQUE,
participant_id VARCHAR NOT NULL,
edipi VARCHAR NOT NULL,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

what is edipi?

@@ -0,0 +1,73 @@
CREATE TABLE IF NOT EXISTS veteran (
veteran_uuid VARCHAR(128) NOT NULL,
PRIMARY KEY(veteran_uuid),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I prefer to have all key declarations at the end

participant_id VARCHAR NOT NULL,
edipi VARCHAR NOT NULL,
createdAt timestamp NOT NULL,
updatedAt timestamp
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can also be NOT NULL

veteran_uuid VARCHAR(128) NOT NULL,
PRIMARY KEY(veteran_uuid),
icn VARCHAR(128) NOT NULL UNIQUE,
participant_id VARCHAR NOT NULL,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

why do some varchars have length and others not?

REFERENCES veteran(veteran_uuid),
id VARCHAR(128) NOT NULL UNIQUE,
id_type VARCHAR NOT NULL UNIQUE,
incoming_status TIMESTAMP,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's a varchar


CREATE TABLE IF NOT EXISTS assessment_result (
assessmentResult_uuid VARCHAR(128) NOT NULL,
claim_uuid VARCHAR(128) NOT NULL,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this. We just need contention_id. These table should match up with the entities in the persistence model.

CREATE TABLE IF NOT EXISTS evidence_summary_document (
esd_uuid VARCHAR(128) NOT NULL,
contention_uuid VARCHAR NOT NULL,
claim_uuid VARCHAR(128) NOT NULL,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This claim_uuid is also gone

CREATE TABLE IF NOT EXISTS claim (
claim_uuid VARCHAR(128) NOT NULL,
PRIMARY KEY(claim_uuid),
veteran_uuid VARCHAR(128) NOT NULL,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

convention should be veteran_id instead of veteran_uuid

CONSTRAINT fk_contention_uuid
FOREIGN KEY (contention_uuid)
REFERENCES contention(contention_uuid),
evidence_count VARCHAR(128) NOT NULL,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is an integer type

CONSTRAINT fk_contention_uuid
FOREIGN KEY (contention_uuid)
REFERENCES contention(contention_uuid),
evidence_count VARCHAR(128) NOT NULL,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is an integer type

@ianwolson ianwolson closed this Jul 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants