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

Enhance/side load images #57

Merged
merged 16 commits into from
Mar 4, 2024
Merged

Enhance/side load images #57

merged 16 commits into from
Mar 4, 2024

Conversation

ajayadav09
Copy link
Contributor

No description provided.

@ajayadav09 ajayadav09 self-assigned this Feb 28, 2024
@ajayadav09 ajayadav09 added the In Progress The Development is currently going on. label Feb 28, 2024
@ajayadav09 ajayadav09 marked this pull request as ready for review March 1, 2024 09:51
@ajayadav09 ajayadav09 added Code Review Code Review and removed In Progress The Development is currently going on. labels Mar 1, 2024
*
* @param array $active_homepage The active homepage data, including block grammar and image URLs.
*/
public static function sideload_and_replace( $active_homepage ) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's make this function generic in order to use it in the future for almost all pages, something like sideload_images_and_replace_grammar ( $page_data ) just a recommendation.

includes/Services/SiteGenService.php Outdated Show resolved Hide resolved
// Upload the images in the 'generatedImages' array to WordPress media library.
$uploaded_image_urls = self::upload_images_to_wp_media_library( $generated_images );

$url_mapping = array_combine( $generated_images, $uploaded_image_urls );
Copy link
Member

Choose a reason for hiding this comment

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

How would the mapping work if say one of the URLs was corrupt or was some link that was not valid say out of 4 images the 3rd one won't now the mapping be:
1 - 1
2 - 2
3 - 4
4 - null

Maybe we can return the array in the upload_images_to_wp_media_library func
It will be a func that takes an array of links and returns a map of old_links -> new_links?

// Update the content with new image URLs.
$active_homepage['content'] = $content;

$data = FlowService::read_data_from_wp_option( false );
Copy link
Member

Choose a reason for hiding this comment

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

I was wondering what if we did something like on Line 482:
'content' => $data['content'],

we will just wrap it like

$content = sideload_images_and_replace_grammar($data['content'], $data['generatedImages'])
'content' => $data['content'],

Basically, I want to just remove the read flow and update flow db call, and make it a bit faster as idk if it will be used later if we save it back in flowData.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this will replace for all the images right? at this point we plan to replace the one which is active.

Copy link
Member

@officiallygod officiallygod left a comment

Choose a reason for hiding this comment

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

Looks Good

@ajayadav09 ajayadav09 added QA Ready for QA and removed Code Review Code Review labels Mar 4, 2024
@diwanshuster diwanshuster merged commit a5e90db into main Mar 4, 2024
@diwanshuster diwanshuster deleted the enhance/side-load-images branch March 4, 2024 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
QA Ready for QA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants