Initial sync can use model methods #56
Conversation
There was a problem hiding this comment.
Impeccable! Thank you!
Let me see how I can have GitHub run the build and I'll release this as patch x.x.X version update. I think this could be considered as a bug fix rather than a feature addition.
Let me know if you believe differently.
I'll also try to upgrade to the newly released gem version from one of my Rails apps which are not on Rails 7.1.x to see if Bundler will try to update and fail or will it stay on the older version until I upgrade to Rails 7.1.x.
| @@ -0,0 +1,2 @@ | |||
| WEBFLOW_API_TOKEN=your_webflow_api_token | |||
| WEBFLOW_SITE_ID=your_webflow_site_id | |||
| actioncable (7.0.4.2) | ||
| actionpack (= 7.0.4.2) | ||
| activesupport (= 7.0.4.2) | ||
| actioncable (7.1.3.3) |
There was a problem hiding this comment.
We should probably upgrade to the lowest supported Rails version...but I don't do it myself either.
I'd leave it how you did it here (don't change anything). We can lower the required version if someone needs it (that could be me as I'm not using Rails 7.1 in any of the apps I'm working on)
| @@ -54,7 +54,7 @@ | |||
|
|
|||
| context "when it shouldn't skip Webflow sync", vcr: { cassette_name: 'webflow_sync/should_skip_webflow_sync' } do | |||
| it 'returns false' do | |||
| expect(article.send(:should_skip_webflow_sync?)).to be false | |||
| expect(article.should_skip_webflow_sync?).to be false | |||
|
Can you please have a look and fix the build issue(s)? |
There was a problem hiding this comment.
Looks like your RubyMine IDE files got checked in as well. Can you please remove them from the PR?
I have this in my ~/.gitignore_global:
*~
.DS_Store
.idea/*
.pry_history
.vscode/*
.yarn/*
!.yarn/patches
!.yarn/releases
!.yarn/plugins
!.yarn/sdks
!.yarn/versions
.pnp.*
/TAGS
*.code-workspace
TODO
I hope it helps.
aedificator-nl
force-pushed
the
feature/initial-sync-should-use-model-settings
branch
from
9820488
to
276acd8
Compare
|
I have fixed some issues. The current issues with the test seem to have to do with the env vars, in test I think |
|
I don't know how to allow PRs from forks to share Github Actions secrets. I tested your changes both locally and I pushed it to a branch on this repo and the build passed. I released this as v9.0.0 due to many gems being updated due to security issues. |
|
Thanks! Great that it all worked out! |
Fixes #55
The initial sync job now uses
should_skip_webflow_sync?andwebflow_collection_idmethods from the model.This way any adjustments to those methods (for example hardcoding a
webflow_collection_idinstead of using theslug) are used in theinitial_sync_jobas well.Also includes a Rails upgrade. This does not seem to break anything, but was needed to get it working locally because of a lagging
nio4rversion.