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 resolve_rendition, instantiating ImageRenditionObjectType by hand?! #330

Closed
wants to merge 4 commits into from

Conversation

engAmirEng
Copy link
Contributor

@engAmirEng engAmirEng commented May 15, 2023

this fixes #318

not sure why they did instantiate the ImageRenditionObjectType by hand instead of returning the Rendition instance in the first place

the faling test is "test_query_rendition_url_field" and there is no way to fix it until #329 is merged

@@ -1005,6 +1005,7 @@ def test_query_url_field(self):
executed["data"]["images"][0]["url"], executed["data"]["images"][0]["src"]
)

@unittest.skip
Copy link
Member

Choose a reason for hiding this comment

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

This indicates that perhaps your changes break intended functionality
Please do not skip tests unless there's a specific reason for it, and when you do you should add a reason for it. ref

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, at least I did it in a separate commit😌
Aside from that, you know this pull request fixes a necessary functionality that grapple claims it has but it doesn't, right?

@zerolab
Copy link
Member

zerolab commented Jun 19, 2023

Thank you for this, @engAmirEng
Closing in favour of #337 which takes care of test as well

@zerolab zerolab closed this Jun 19, 2023
@engAmirEng
Copy link
Contributor Author

Thank you for this, @engAmirEng
Closing in favour of #337 which takes care of test as well

Happy to see the repo is going forward🥳

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.

querying for "custom_rendition_property" in the example, results in error
2 participants