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

Rename the accredited_provider_id column #4712

Merged

Conversation

inulty-dfe
Copy link
Contributor

@inulty-dfe inulty-dfe commented Nov 26, 2024

Context

Provider model has a column named accredited_provider_id.
The value represents an identifier used by Funding and Market Regs to discriminate between providers with similar names and such.

_id suffixes in Ruby on Rails conventionally denote a foreign key columns on the model.

We're changing this column name to avoid obvious misunderstandings.

Changes proposed in this pull request

Rename accredited_provider_id column to accredited_provider_number

Guidance to review

@inulty-dfe inulty-dfe requested a review from a team as a code owner November 26, 2024 12:16
@inulty-dfe
Copy link
Contributor Author

@inulty-dfe inulty-dfe force-pushed the im/267-tech-debt-rename-the-accreditedproviderid-column branch 3 times, most recently from 798e734 to 8ba9e69 Compare November 26, 2024 16:28
@inulty-dfe inulty-dfe added deploy A Review App will be created for PRs with this label and removed deploy A Review App will be created for PRs with this label labels Nov 26, 2024
Copy link
Contributor

@Nitemaeric Nitemaeric left a comment

Choose a reason for hiding this comment

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

Still seeing accredited_provider_id in the codebase was a surprise. 😅 Now accredited_provider_id is actually what we thought it would be - ish - an AR id!

All good! Nice one.

@inulty-dfe
Copy link
Contributor Author

Still seeing accredited_provider_id in the codebase was a surprise. 😅 Now accredited_provider_id is actually what we thought it would be - ish - an AR id!

All good! Nice one.

Yeah I was also confused by it's existence. It's a param in the AccreditedProvidersController. I'd expect that just to be id. But we move on.

@inulty-dfe inulty-dfe force-pushed the im/267-tech-debt-rename-the-accreditedproviderid-column branch from 7f190c0 to a465d8c Compare November 27, 2024 13:41
  The AccreditedProvidersController still uses a param called
  `accredited_provdier_id`. It is unrelated to the
  accredited_provider_number
@inulty-dfe inulty-dfe force-pushed the im/267-tech-debt-rename-the-accreditedproviderid-column branch from a465d8c to 843da5d Compare November 27, 2024 13:48
@inulty-dfe inulty-dfe merged commit b843956 into main Nov 27, 2024
19 checks passed
@inulty-dfe inulty-dfe deleted the im/267-tech-debt-rename-the-accreditedproviderid-column branch November 27, 2024 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deploy A Review App will be created for PRs with this label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants