-
Notifications
You must be signed in to change notification settings - Fork 40
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 migrate from storyblok id to storyblok UUID #786
Feat migrate from storyblok id to storyblok UUID #786
Conversation
Hi @kyleecodes So I tried my best to make all the changes and I think I have covered them all here. I also updated the test cases wherever it was required. Since this required making changes in test cases also, I am not sure about whatever I have done is correct or not. So if possible can you please take a look at my PR? if it looks ok, then I will start making changes in the frontend repo and will run the full unit tests as well as e2e tests there using playwright. |
@ApiProperty({ type: Number }) | ||
story_id: number; | ||
@ApiProperty({ type: String }) | ||
story_uuid: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm in contact with Storyblok support because I checked the webhook and realised they only send story_id at the moment. I'm wondering whether its a config change on our side or whether they just haven't migrated their webhook functionality to use storyblokUuid yet! Will keep you posted.
Hey thanks for this! After an initial look, most of it looks great. I realised that I hadn't checked the changes in the webhook to see what storyblok was sending instead of story_id. It seems like they haven't changed to sending the uuid ryet in the body of the webhook. I am going to follow up with support and let you know what they say and then I'll finish my review after I get the answer from them. Hope thats okay! |
Thanks for the review. I'll update this PR to revert the webhook to use the story_id in sometime. |
Hi @eleanorreem , I have a doubt regarding keeping the storyblokId in the webhook. From my understanding, what I can see is that in the function So whatever actions we are performing in that file, we need to use But in the functions in that service, we make calls to |
Hi @strawHat121 I wasn't clear apologies. I agree with you. Keep both for now in the webhook and use storyblokUuid elsewhere! And then when they fully migrate away from the legacy storyblokId, we can remove it from our code base. |
Hi @eleanorreem I have reverted the changes in the webhook service and I have kept both I have made changes in the tests as well. But few tests are failing (and these are also failing in develop) because the Apart from this I tried my best to make the changes. Let me know if any more changes are required. |
Thanks @strawHat121 all merged ⭐ |
Resolves #781
What changes did you make and why did you make them?
storyblokId
and replaced it withstoryblokUuid
Did you run tests? Share screenshot of results:
How did you find us? (GitHub, Google search, social media, etc.):
Github