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

Fix #611: ContactSchema now reflects what is stored in database #711

Merged
merged 8 commits into from
Jun 28, 2023

Conversation

leplatrem
Copy link
Contributor

@leplatrem leplatrem commented Jun 26, 2023

This PR clarifies a bit that we are sometimes dealing with instances coming the database (*Table* schemas) and instances coming as input (*In* schemas).

It fixes bug #611 where timestamps were dropped during the Acoustinc sync process (because objects from the database were casted to input objects that don't have timestamp fields)

@leplatrem leplatrem force-pushed the 611-models-acoustic-sync branch 2 times, most recently from f41c77a to 1696c47 Compare June 26, 2023 14:49
@leplatrem leplatrem added the enhancement New feature or request label Jun 26, 2023
@leplatrem leplatrem force-pushed the 611-models-acoustic-sync branch from 1696c47 to 4c4c853 Compare June 26, 2023 15:03
@leplatrem leplatrem marked this pull request as ready for review June 26, 2023 15:16
@leplatrem leplatrem requested a review from a team as a code owner June 26, 2023 15:16
@leplatrem leplatrem added internal-change and removed enhancement New feature or request labels Jun 26, 2023
Copy link
Contributor

@grahamalama grahamalama left a comment

Choose a reason for hiding this comment

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

instances coming the database (*Table* schemas)

I saw this and was slightly thrown by the naming introduced in d0e27b0, since there's no "Contact" table in the database. Is this to mean "a contact comprised entirely of *TableSchema objects?

ctms/crud.py Outdated Show resolved Hide resolved
ctms/crud.py Outdated Show resolved Hide resolved
@leplatrem leplatrem requested a review from grahamalama June 27, 2023 13:57
Copy link
Contributor

@grahamalama grahamalama left a comment

Choose a reason for hiding this comment

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

Looks good! Some of the testing methodology does pain me, but hopefully we'll continue to make improvements here as we improve our schemas

ctms/crud.py Outdated Show resolved Hide resolved
ctms/crud.py Outdated Show resolved Hide resolved
leplatrem and others added 2 commits June 28, 2023 14:23
Co-authored-by: grahamalama <gbeckley@mozilla.com>
@leplatrem leplatrem merged commit 1342217 into main Jun 28, 2023
@leplatrem leplatrem deleted the 611-models-acoustic-sync branch June 28, 2023 13:54
leplatrem added a commit that referenced this pull request Jun 29, 2023
leplatrem added a commit that referenced this pull request Jun 29, 2023
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.

2 participants