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

GMB Details & Hours #44 #55

Merged
merged 37 commits into from
Mar 6, 2021
Merged

GMB Details & Hours #44 #55

merged 37 commits into from
Mar 6, 2021

Conversation

chx2
Copy link
Contributor

@chx2 chx2 commented Mar 1, 2021

  • GmbDetails Model/Factory/Policy/Migration
  • GmbHours Model/Factory/Policy/Migration
  • Needs tipoff/support to tag a new release

@drewroberts

This comment has been minimized.

@drewroberts

This comment has been minimized.

@chx2 chx2 added the question Further information is requested label Mar 2, 2021
@chx2
Copy link
Contributor Author

chx2 commented Mar 2, 2021

@drewroberts I recall that we want a model for Google MyBusiness entries; is there a specific reason why there's two tables for GMB data? It looks like both hold references to a Location model.

Would it make sense compress the tables by storing daily open/close status values via a jsonb column into the gmb_details table? Or is there something I'm missing?

@drewroberts
Copy link
Member

Let's create 2 separate models for the Google My Business Data. You can call them GmbDetails and GmbHours

I like having them separate because I want to know when was the last time the main Details about the business in GMB (#44) was updated vs just an update to the hours of operation.

I thought about using jsonb for the values as that would be better in most cases, but since I am trying to essentially version control when they were changed, I prefer using actual table fields here.

@drewroberts
Copy link
Member

drewroberts commented Mar 4, 2021

I'm going to remove Timezone from this PR and move it over to the tipoff/addresses package.

@drewroberts
Copy link
Member

I moved Timezones from this PR to the one in the tipoff/addresses package:

@drewroberts drewroberts self-requested a review March 4, 2021 18:30
@chx2
Copy link
Contributor Author

chx2 commented Mar 4, 2021

Added the new models/factories/migrations/policies. Need to add aliases to support to fix tests

@chx2
Copy link
Contributor Author

chx2 commented Mar 4, 2021

TIPOFF/support#77

@chx2 chx2 removed the question Further information is requested label Mar 4, 2021
@chx2 chx2 marked this pull request as ready for review March 4, 2021 23:47
@drewroberts
Copy link
Member

I think I didn't communicate well in issue #52 about Profile Links. That model is supposed to go into the tipoff/seo package.

@chx2
Copy link
Contributor Author

chx2 commented Mar 6, 2021

@drewroberts is there anything else that needs to be done in this PR?

@drewroberts drewroberts changed the title Refactored Models GMB Details & Hours #44 Mar 6, 2021
Copy link
Member

@drewroberts drewroberts left a comment

Choose a reason for hiding this comment

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

Thank you! 🔭

@drewroberts drewroberts merged commit ae3b1d0 into main Mar 6, 2021
@drewroberts drewroberts deleted the chx2/feature/refactored-models branch March 6, 2021 20:18
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