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

feat: add user vat id #940

Merged
merged 2 commits into from
Jan 12, 2024
Merged

feat: add user vat id #940

merged 2 commits into from
Jan 12, 2024

Conversation

asadali145
Copy link
Contributor

@asadali145 asadali145 commented Dec 5, 2023

What are the relevant tickets?

Closes mitodl/mitxpro#2762

Description (What does it do?)

Adds user vat id.

How can this be tested?

  • Run the following and verify that vat_id is available.
  • dbt build -s +stg__mitxpro__app__postgres__users_legaladdress --vars 'schema_suffix: asad' --target dev_production
  • dbt build -s +int__mitxpro__users --vars 'schema_suffix: asad' --target dev_production

@asadali145
Copy link
Contributor Author

I am not able to refresh the schema. Also, getting an error when I try to test the models locally. Is there anything that I am missing?
Screen Shot 2024-01-11 at 2 25 39 PM
Screen Shot 2024-01-11 at 2 26 08 PM

@asadali145 asadali145 marked this pull request as ready for review January 12, 2024 06:54
Copy link
Contributor

@rachellougee rachellougee left a comment

Choose a reason for hiding this comment

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

The new field should also add to source file https://github.com/mitodl/ol-data-platform/blob/main/src/ol_dbt/models/staging/mitxpro/_mitxpro__sources.yml#L853

Other than small comment for field description, it looks good to me

@@ -944,6 +944,8 @@ models:
description: string, user first name
tests:
- not_null
- name: user_vat_id
description: string, user vat id
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add some description on what user vat id is? I think its useful to document that.

Copy link
Contributor

@rachellougee rachellougee left a comment

Choose a reason for hiding this comment

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

👍

@asadali145 asadali145 merged commit d09f73f into main Jan 12, 2024
2 checks passed
@asadali145 asadali145 deleted the asad/add-user-vat-id branch January 12, 2024 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add VAT number to User Profile
2 participants