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

Ensure uploaded image is inserted with correct size #323

Merged
merged 8 commits into from
Jan 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@
"upload_request",
"[a-z]+_outputFormat",
"[a-z]+_quality",
"gif_convert"
"gif_convert",
"media_details"
]
}
]
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/e2e-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ jobs:
COLLECT_COVERAGE: ${{ matrix.wp == 'latest' }}

- name: Upload code coverage report
uses: codecov/[email protected].4
uses: codecov/[email protected].5
with:
file: artifacts/e2e-coverage/coverage/lcov.info
flags: e2e
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/lint-and-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ jobs:
run: npm run test:unit -- --collectCoverage

- name: Upload code coverage report
uses: codecov/[email protected].4
uses: codecov/[email protected].5
with:
file: artifacts/logs/lcov.info
flags: js
Expand Down Expand Up @@ -212,14 +212,14 @@ jobs:
if: ${{ matrix.coverage }}

- name: Upload code coverage report
uses: codecov/[email protected].4
uses: codecov/[email protected].5
with:
file: artifacts/coverage.xml
flags: default
if: ${{ matrix.coverage }}

- name: Upload code coverage report
uses: codecov/[email protected].4
uses: codecov/[email protected].5
with:
file: coverage-multisite.xml
flags: multisite
Expand Down
98 changes: 70 additions & 28 deletions inc/class-rest-attachments-controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,9 @@
* generate_sub_sizes?: bool,
* }
* @phpstan-type SideloadRequest array{
* post?: int,
* image_size?: string,
* id: int,
* image_size: string,
* upload_request?: string,
* }
*/
class REST_Attachments_Controller extends WP_REST_Attachments_Controller {
Expand All @@ -45,7 +46,7 @@
$args['generate_sub_sizes'] = [
'type' => 'boolean',
'default' => true,
'description' => __( 'Whether to generate image sub sizes.' ),
'description' => __( 'Whether to generate image sub sizes.', 'media-experiments' ),

Check warning on line 49 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L49

Added line #L49 was not covered by tests
];
$args['upload_request'] = [
'description' => __( 'Upload request this file is for.', 'media-experiments' ),
Expand Down Expand Up @@ -74,18 +75,24 @@
true
);

// TODO: Consider support general sideloading, not attached to any post.
register_rest_route(
$this->namespace,
'/' . $this->rest_base . '/sideload',
'/' . $this->rest_base . '/(?P<id>[\d]+)/sideload',

Check warning on line 81 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L81

Added line #L81 was not covered by tests
[
[
'methods' => WP_REST_Server::CREATABLE,
'callback' => [ $this, 'sideload_item' ],
'permission_callback' => [ $this, 'create_item_permissions_check' ],
'permission_callback' => [ $this, 'sideload_item_permissions_check' ],

Check warning on line 86 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L86

Added line #L86 was not covered by tests
'args' => [
'id' => array(
'description' => __( 'Unique identifier for the attachment.', 'media-experiments' ),
'type' => 'integer',
),

Check warning on line 91 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L88-L91

Added lines #L88 - L91 were not covered by tests
'image_size' => [
'description' => __( 'Image size.', 'media-experiments' ),
'type' => 'string',
'required' => true,

Check warning on line 95 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L95

Added line #L95 was not covered by tests
],
'upload_request' => [
'description' => __( 'Upload request this file is for.', 'media-experiments' ),
Expand All @@ -110,7 +117,7 @@

$params['upload_request'] = [
'default' => null,
'description' => __( 'Limit result set to attachments associated with a given attachment request.' ),
'description' => __( 'Limit result set to attachments associated with a given attachment request.', 'media-experiments' ),

Check warning on line 120 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L120

Added line #L120 was not covered by tests
'type' => 'string',
];

Expand Down Expand Up @@ -333,7 +340,7 @@
if ( ! empty( $request['id'] ) ) {
return new WP_Error(
'rest_post_exists',
__( 'Cannot create existing post.' ),
__( 'Cannot create existing post.', 'media-experiments' ),

Check warning on line 343 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L343

Added line #L343 was not covered by tests
[ 'status' => 400 ]
);
}
Expand All @@ -343,47 +350,47 @@
if ( ! $post_type ) {
return new WP_Error(
'rest_cannot_edit_others',
__( 'Sorry, you are not allowed to create posts as this user.' ),
__( 'Sorry, you are not allowed to create posts as this user.', 'media-experiments' ),

Check warning on line 353 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L353

Added line #L353 was not covered by tests
[ 'status' => rest_authorization_required_code() ]
);
}

if ( ! empty( $request['author'] ) && get_current_user_id() !== $request['author'] && ! current_user_can( $post_type->cap->edit_others_posts ) ) {
return new WP_Error(
'rest_cannot_edit_others',
__( 'Sorry, you are not allowed to create posts as this user.' ),
__( 'Sorry, you are not allowed to create posts as this user.', 'media-experiments' ),

Check warning on line 361 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L361

Added line #L361 was not covered by tests
[ 'status' => rest_authorization_required_code() ]
);
}

if ( ! empty( $request['sticky'] ) && ! current_user_can( $post_type->cap->edit_others_posts ) && ! current_user_can( $post_type->cap->publish_posts ) ) {
return new WP_Error(
'rest_cannot_assign_sticky',
__( 'Sorry, you are not allowed to make posts sticky.' ),
__( 'Sorry, you are not allowed to make posts sticky.', 'media-experiments' ),

Check warning on line 369 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L369

Added line #L369 was not covered by tests
[ 'status' => rest_authorization_required_code() ]
);
}

if ( ! current_user_can( $post_type->cap->create_posts ) && ! $this->is_valid_upload_request( $request ) ) {
return new WP_Error(
'rest_cannot_create',
__( 'Sorry, you are not allowed to create posts as this user.' ),
__( 'Sorry, you are not allowed to create posts as this user.', 'media-experiments' ),

Check warning on line 377 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L377

Added line #L377 was not covered by tests
[ 'status' => rest_authorization_required_code() ]
);
}

if ( ! $this->check_assign_terms_permission( $request ) ) {
return new WP_Error(
'rest_cannot_assign_term',
__( 'Sorry, you are not allowed to assign the provided terms.' ),
__( 'Sorry, you are not allowed to assign the provided terms.', 'media-experiments' ),

Check warning on line 385 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L385

Added line #L385 was not covered by tests
[ 'status' => rest_authorization_required_code() ]
);
}

if ( ! current_user_can( 'upload_files' ) && ! $this->is_valid_upload_request( $request ) ) {
return new WP_Error(
'rest_cannot_create',
__( 'Sorry, you are not allowed to upload media on this site.' ),
__( 'Sorry, you are not allowed to upload media on this site.', 'media-experiments' ),

Check warning on line 393 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L393

Added line #L393 was not covered by tests
[ 'status' => 400 ]
);
}
Expand All @@ -400,7 +407,7 @@
) {
return new WP_Error(
'rest_cannot_edit',
__( 'Sorry, you are not allowed to upload media to this post.' ),
__( 'Sorry, you are not allowed to upload media to this post.', 'media-experiments' ),

Check warning on line 410 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L410

Added line #L410 was not covered by tests
[ 'status' => rest_authorization_required_code() ]
);
}
Expand Down Expand Up @@ -453,6 +460,41 @@
return $posts[0];
}

/**
* Checks if a given request has access to sideload a file.
*
* Sideloading a file for an existing attachment
* requires both update and create permissions.
*
* @param WP_REST_Request $request Full details about the request.
* @return true|WP_Error True if the request has access to update the item, WP_Error object otherwise.
*/
public function sideload_item_permissions_check( $request ): WP_Error|bool {
$post = $this->get_post( $request['id'] );

Check warning on line 473 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L472-L473

Added lines #L472 - L473 were not covered by tests

if ( is_wp_error( $post ) ) {
return $post;

Check warning on line 476 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L475-L476

Added lines #L475 - L476 were not covered by tests
}

if ( ! $this->check_update_permission( $post ) && ! $this->is_valid_upload_request( $request ) ) {
return new WP_Error(
'rest_cannot_edit',
__( 'Sorry, you are not allowed to edit this post.', 'media-experiments' ),
array( 'status' => rest_authorization_required_code() )
);

Check warning on line 484 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L479-L484

Added lines #L479 - L484 were not covered by tests
}

if ( ! current_user_can( 'upload_files' ) && ! $this->is_valid_upload_request( $request ) ) {
return new WP_Error(
'rest_cannot_create',
__( 'Sorry, you are not allowed to upload media on this site.', 'media-experiments' ),
[ 'status' => 400 ]
);

Check warning on line 492 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L487-L492

Added lines #L487 - L492 were not covered by tests
}

return true;

Check warning on line 495 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L495

Added line #L495 was not covered by tests
}

/**
* Side-loads a media file without creating an attachment.
*
Expand All @@ -461,7 +503,9 @@
* @phpstan-param WP_REST_Request<SideloadRequest> $request
*/
public function sideload_item( WP_REST_Request $request ): WP_Error|WP_REST_Response {
if ( ! empty( $request['post'] ) && 'attachment' !== get_post_type( $request['post'] ) ) {
$attachment_id = $request['id'];

Check warning on line 506 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L506

Added line #L506 was not covered by tests

if ( 'attachment' !== get_post_type( $attachment_id ) ) {

Check warning on line 508 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L508

Added line #L508 was not covered by tests
return new WP_Error(
'rest_invalid_param',
__( 'Invalid parent type.', 'media-experiments' ),
Expand All @@ -479,7 +523,6 @@
// See https://github.com/WordPress/wordpress-develop/blob/30954f7ac0840cfdad464928021d7f380940c347/src/wp-includes/functions.php#L2576-L2582
// With this filter we can work around this safeguard.

$attachment_id = $request['post'];
$attachment_filename = get_attached_file( $attachment_id, true );
$attachment_filename = $attachment_filename ? basename( $attachment_filename ) : null;

Expand Down Expand Up @@ -530,12 +573,11 @@
return $file;
}

$url = $file['url'];
$type = $file['type'];
$path = $file['file'];

// TODO: Better fallback if image_size is not provided.
$image_size = $request['image_size'] ?? 'thumbnail';
$image_size = $request['image_size'];

Check warning on line 580 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L580

Added line #L580 was not covered by tests

$metadata = wp_get_attachment_metadata( $attachment_id, true );

Expand All @@ -561,17 +603,17 @@

wp_update_attachment_metadata( $attachment_id, $metadata );

$data = [
'success' => true,
'url' => $url,
];

$response = rest_ensure_response( $data );
$response = $this->prepare_item_for_response(
get_post( $attachment_id ),

Check warning on line 607 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L606-L607

Added lines #L606 - L607 were not covered by tests
// TODO: Maybe forward context or _fields param?
new WP_REST_Request(
WP_REST_Server::READABLE,
rest_url( sprintf( '%s/%s/%d', $this->namespace, $this->rest_base, $attachment_id ) )
)
);

Check warning on line 613 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L609-L613

Added lines #L609 - L613 were not covered by tests

if ( ! is_wp_error( $response ) ) {
$response->set_status( 201 );
$response->header( 'Location', rest_url( sprintf( '%s/%s/%d', $this->namespace, $this->rest_base, $attachment_id ) ) );
}
$response->set_status( 201 );
$response->header( 'Location', rest_url( sprintf( '%s/%s/%d', $this->namespace, $this->rest_base, $attachment_id ) ) );

Check warning on line 616 in inc/class-rest-attachments-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-attachments-controller.php#L615-L616

Added lines #L615 - L616 were not covered by tests

if ( function_exists( 'perflab_server_timing_register_metric' ) ) {
perflab_server_timing_register_metric(
Expand Down
40 changes: 21 additions & 19 deletions inc/class-rest-upload-requests-controller.php
Original file line number Diff line number Diff line change
Expand Up @@ -56,12 +56,6 @@
'type' => 'string',
],
],
[
'methods' => WP_REST_Server::READABLE,
'callback' => [ $this, 'get_item' ],
'permission_callback' => [ $this, 'get_item_permissions_check' ],
'args' => $get_item_args,
],
[
'methods' => WP_REST_Server::DELETABLE,
'callback' => [ $this, 'delete_item' ],
Expand All @@ -74,17 +68,14 @@
}

/**
* Retrieves a single post.
* Gets an upload request by its slug.
*
* @since 4.7.0
*
* @param WP_REST_Request $request Full details about the request.
* @return WP_REST_Response|WP_Error Response object on success, or WP_Error object on failure.
* @phpstan-param WP_REST_Request<array{slug: string}> $request
* @param string $slug Supplied slug.
* @return WP_Post|WP_Error Post object if slug is valid, WP_Error otherwise.
*/
public function get_item( $request ) {
protected function get_post( $slug ) {

Check warning on line 76 in inc/class-rest-upload-requests-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-upload-requests-controller.php#L76

Added line #L76 was not covered by tests
$args = [
'name' => $request['slug'],
'name' => $slug,

Check warning on line 78 in inc/class-rest-upload-requests-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-upload-requests-controller.php#L78

Added line #L78 was not covered by tests
'post_type' => 'mexp-upload-request',
'post_status' => 'publish',
'numberposts' => 1,
Expand All @@ -101,7 +92,22 @@
);
}

$post = $posts[0];
return $posts[0];

Check warning on line 95 in inc/class-rest-upload-requests-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-upload-requests-controller.php#L95

Added line #L95 was not covered by tests
}

/**
* Retrieves a single post.
*
* @param WP_REST_Request $request Full details about the request.
* @return WP_REST_Response|WP_Error Response object on success, or WP_Error object on failure.
* @phpstan-param WP_REST_Request<array{slug: string}> $request
*/
public function get_item( $request ) {
$post = $this->get_post( $request['slug'] );

Check warning on line 106 in inc/class-rest-upload-requests-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-upload-requests-controller.php#L105-L106

Added lines #L105 - L106 were not covered by tests

if ( is_wp_error( $post ) ) {
return $post;

Check warning on line 109 in inc/class-rest-upload-requests-controller.php

View check run for this annotation

Codecov / codecov/patch

inc/class-rest-upload-requests-controller.php#L108-L109

Added lines #L108 - L109 were not covered by tests
}

$data = $this->prepare_item_for_response( $post, $request );
$response = rest_ensure_response( $data );
Expand All @@ -121,8 +127,6 @@
/**
* Retrieves the post's schema, conforming to JSON Schema.
*
* @since 4.7.0
*
* @return array Item schema data.
* @phpstan-return array<string,mixed>
*/
Expand Down Expand Up @@ -201,8 +205,6 @@
* - `rest_page_item_schema`
* - `rest_attachment_item_schema`
*
* @since 5.4.0
*
* @param array $schema Item schema data.
*/
$schema = apply_filters( "rest_{$this->post_type}_item_schema", $schema ); // phpcs:ignore WordPress.NamingConventions.PrefixAllGlobals.NonPrefixedHooknameFound
Expand Down
Loading
Loading