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

Image asset complex field handler #203

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

erdnaxelaweb
Copy link

Add field handler for the ezplatform ezimageasset field type

=> Add ezimageasset as new fieldhandler for kaliop migration bundle
@codecov-io
Copy link

codecov-io commented May 21, 2019

Codecov Report

Merging #203 into master will decrease coverage by 0.13%.
The diff coverage is 12.5%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #203      +/-   ##
============================================
- Coverage     67.69%   67.56%   -0.14%     
- Complexity     2578     2585       +7     
============================================
  Files           123      124       +1     
  Lines          6714     6730      +16     
============================================
+ Hits           4545     4547       +2     
- Misses         2169     2183      +14
Impacted Files Coverage Δ Complexity Δ
Core/FieldHandler/EzImageAsset.php 12.5% <12.5%> (ø) 7 <7> (?)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4019189...49ec65a. Read the comment docs.

@gggeek
Copy link
Member

gggeek commented May 22, 2019

Nice, thanks.
Would you be able to add a test case as well? A yml migration file that creates a content type with an ezimageasset field, then a content, then removes both, in tests/dsl/ezimageasset/ would be enough - I can add the phpunit part.

@erdnaxelaweb
Copy link
Author

I will let @mpoudevigne answer as he was the one working on this :)

@mpoudevigne
Copy link

@gggeek Can you say me if it's ok my kaliop migration files committed ? thanks

@gggeek
Copy link
Member

gggeek commented May 22, 2019

@mpoudevigne yes, migration files are committed ok. I hope to merge and release soon

@mpoudevigne
Copy link

@gggeek thanks

// 2. resolve remote ids
$id = $this->contentMatcher->matchOneByKey($id)->id;

return new Value($id, $altText);
Copy link
Contributor

Choose a reason for hiding this comment

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

Are you sure that the $id is int? I had similar issues when using references https://github.com/ezsystems/ezplatform-ee-demo/blob/v2.5.1/src/AppBundle/Migration/FieldHandler/EzImageAsset.php#L37

is your solution works with content exporter command?

Copy link
Member

Choose a reason for hiding this comment

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

ping @florianalexandre any answer?

Copy link

Choose a reason for hiding this comment

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

Any plans to merge this?

@gggeek gggeek changed the base branch from master to main October 13, 2022 08:17
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.

6 participants