Upgrade to Webflow API v2 #23
Conversation
…anges live until the next publish
…mfile`; Remove VCR cassettes; v2.0.0
- update GitHub action - Move TEST_API_TOKEN from codebase - Add 'rubocop' and 'rubocop-minitest' gems - Setup default Rake task to run both RuboCop and minitest tests - Fix a few client issues; Don't rely on ActiveSupport's `Array.wrap` - Fix all tests - Fix error class
|
Hi Viktor, I will try to review this as soon as possible and add you as a maintainer so that you can publish a new mayor version. Maybe there is a way to support both API versions? Cheers |
|
Ah, I'm actually no longer maintainer of this repo 😸 Let @sega handle it. |
|
|
||
| require 'webflow' | ||
|
|
||
| TEST_API_TOKEN = '1f0da5c9368af9cb2dcd65d22a6600a8ffa069f70729e129a09787203bc2c2be' |
There was a problem hiding this comment.
This is intentional, dont remove the test token. Use it as a fallback throughout.
There was a problem hiding this comment.
I couldn't get a single API query to return anything other than:
/Users/viktor/Developer/gems/webflow-ruby/lib/webflow/client.rb:152:in `request': Request not authorized (Webflow::Error)
That's why I left it to the user (developer) to pass in their own working API token.
There was a problem hiding this comment.
What's the use of the token that doesn't work?
There was a problem hiding this comment.
hm, looks like it has vanished... 🤷 at least i fixed my blog while looking for it 😆
lib/webflow/client.rb
Outdated
| result | ||
| end | ||
|
|
||
| def update_item(collection_id, item_id, data, is_archived: false, is_draft: false, publish: false) # rubocop:disable Metrics/ParameterLists |
There was a problem hiding this comment.
increase the limit or disable this rule instead please.
There was a problem hiding this comment.
Hmmm, this rule has those limitations for a reason.
https://docs.rubocop.org/rubocop/cops_metrics.html#metricsparameterlists
I believe the default limit of 5 is a good limit, maybe even too generous/high. It was only disabled here in one place. I find that to be better than increasing the limit globally to 6, which makes this rule's function almost useless, and allows developers to easily add one more argument, as there's no "warning" about having too many arguments.
https://thoughtbot.com/blog/sandi-metz-rules-for-developers#four-method-arguments
I think disabling rules inline is fine. It stands right there in the code, in your face, so that you can see it and try to figure out a way to refactor your code so that you can remove the inline disable.
But if you really have strong opinion, perhaps we could add CountKeywordArgs: false and we could remove the inline disable?
There was a problem hiding this comment.
i dislike 90% of the preconfigured rubocop rules, so please ignore me if you feel different about it. i just really hate cluttering the code for a tool. that should generally be forbidden.
| end | ||
|
|
||
| def create_item(collection_id, data, live: false) | ||
| post("/collections/#{collection_id}/items", {fields: data}, live: live) | ||
| def delete_item(collection_id, item_id) |
There was a problem hiding this comment.
If it's deleted, it's pretty much unpublished, since it's actually gone. This is Webflow API internal bug that we cannot fix. And this is an implementation detail of how delete_item works. I don't think the user of webflow-ruby needs to know all the workarounds we need to implement. They just want to know the API & methods of the gem so that they can use them. delete_item to me sounds much clearer than delete_and_unpublish.
There was a problem hiding this comment.
yeah, that's probably true. as long as the underlying primites are still exposed and one can implement something more custom i'm fine with it.
There was a problem hiding this comment.
Ahhhh, I see...
I wasn't keeping in consideration supporting the underlying low-level API access.
In this case, we lost the original "delete item" low-level API access.
Since it wasn't working as expected in the first place, I'm not sure if we really lost anything?
I "lost" a lot of time debugging and figuring out why items with publish: true (ex live: true) aren't visible on the live site. The (still not fixed) Webflow bug causes changes to not being published anymore for any updates to any items, after the buggy "delete item" request is made.
I'm just thinking of the use-case where someone might need the low-level access to the API? Perhaps if there's more functionality added to the API and the gem doesn't get updated regularly / on time.
Should we just expose the access to request and add a comment saying that it's a low-level API and there most likely exists a higher-level method that can be used instead?
Or if you want to add low-level (buggy) delete_item, I'd vote for keeping the working, higher-level delete_item as-is and add a new method delete_item_single / delete_item_raw / delete_item_something with the comment explaining why there are two methods and which one works / is recommended to be used.
What do you think?
There was a problem hiding this comment.
just re-reading the documentation and your comments, i think this should definitively be an additional method.
is it correct that delete_item works fine if you publish the site afterwards? bc, that is what i would have expected in the first place.
There was a problem hiding this comment.
Publishing a website is discouraged and has a special very low limit of 1 call every 60 seconds:
https://developers.webflow.com/changelog/publish-site-api-new-rate-limits
It's also not the same as publishing a single item (whether it's create, update or delete). Publishing, publishes all of the pending work in design + pending collection items.
But you're right that we are missing "mark for deletion" (delete, pending).
It's tricky to test delete pending as I'm not sure whether it would work or the mentioned bug would happen. The issue is that, if I remember correctly, the API returns everything as it should be, but on your actual Webflow site, you can see that the new changes are not being applied. So it's impossible to write an automated test to test this (the automated test would have to navigate to a Webflow site and check that there's no such item on the page).
If you have time please take over and feel free to look into this and add the missing functionality.
Otherwise, I can add a comment about delete pending missing in the gem and invite people to add it if someone needs it. Unfortunately, I don't have the time right now to look into this buggy functionality and try to figure out in which cases it works in which cases it doesn't.
There was a problem hiding this comment.
i dont really understand the webflow API versioning here, is the "v2 BETA" before or after "v2"? well at least there is another method for delete and publish https://docs.developers.webflow.com/v2.0.0-beta/reference/delete-item-live
There was a problem hiding this comment.
I think it started as API v2 beta and then they released API v2 and "forgot" to update API v2 beta to API beta or API v3 beta.
I totally missed that they've added these live methods for create, update, and delete. I assume this will be a part of API v3.
When they launch API v3, I'll try to keep the gem updated.
|
Hey @phoet thanks for the review! I replied to your comments. |
API v1 is legacy: So I don't see the need to support it. |
|
Hey @phoet, how's it going? Let me know if you're waiting for anything from my side. Re live parameters / endpoints, those are on unstable API so I wouldn't add those to this version. Re rubocop directives, I can remove those, maybe switch to standardrb... |
|
Hey @sega, do you have any time to look into this? |
|
I figured it's gonna be easier if I just release this as a new gem. I've refactored / rewritten many parts of the gem and made it more opinionated. It no longer aligns with the webflow-ruby philosophy of providing the "bare metal", it's much more opinionated now (for better or for worse). Perhaps webflow-ruby should stay the way it is. The idea of the new gem webflow-rb is to offer higher level API so that it can just be used. |
|
Sorry that you got no reply from anyone in the @penseo org! Thanks for taking the time trying to contribute to upstream. |
0 public Members 😆 |
|
|
|
I happen to know that they have at least 2 programmers on staff. 🤷 |
Hi @penseo!
I've spent some time to upgrade the client to Webflow API v2. I've also added a few gems to help with development and testing.
I've fixed and re-recorded all VCR cassettes of all the tests so that they all pass.
Let me know how you'd like to proceed?
Should I fork the gem, call it differently and publish it to RubyGems under a new name or can you have a look or can you make me a maintainer?
Seems like the command to add a maintainer to be able to push to RubyGems is
gem owner [GEM NAME] --add [USER EMAIL]My RubyGems email is viktor.fonic [at] gmail.com
Thanks!
Viktor