diff --git a/changelog/8066-import-dedup-by-source-id b/changelog/8066-import-dedup-by-source-id new file mode 100644 index 0000000000..ea49c1b2a1 --- /dev/null +++ b/changelog/8066-import-dedup-by-source-id @@ -0,0 +1,4 @@ +Significance: patch +Type: fixed + +Fix duplicate posts when re-importing content with empty slug columns by persisting the source import id on each imported post. \ No newline at end of file diff --git a/config/psalm/psalm-baseline.xml b/config/psalm/psalm-baseline.xml index 91f6961fb5..0d1b919fb9 100644 --- a/config/psalm/psalm-baseline.xml +++ b/config/psalm/psalm-baseline.xml @@ -4416,6 +4416,10 @@ + + task]]> + task]]> + @@ -4425,14 +4429,19 @@ + + + task]]> task]]> + task]]> + diff --git a/includes/data-port/models/class-sensei-data-port-schema.php b/includes/data-port/models/class-sensei-data-port-schema.php index eb8348a65b..27b1bc7151 100644 --- a/includes/data-port/models/class-sensei-data-port-schema.php +++ b/includes/data-port/models/class-sensei-data-port-schema.php @@ -14,6 +14,18 @@ abstract class Sensei_Data_Port_Schema { const COLUMN_ID = 'id'; const COLUMN_SLUG = 'slug'; + /** + * Post meta key used to record the source import id on every imported post. + * + * Provides durable per-post identity so re-imports of the same source row + * resolve to the existing post even when the slug is empty. + * + * @since $$next-version$$ + * + * @var string + */ + const META_KEY_IMPORT_ID = '_sensei_import_id'; + /** * Post statuses supported by the data port. Export coerces any other status to * `draft`, and import only accepts these; keeping both sides in sync avoids a @@ -73,6 +85,17 @@ public function get_column_slug() { return self::COLUMN_SLUG; } + /** + * Get the post meta key used to record the source import id. + * + * @since $$next-version$$ + * + * @return string + */ + public function get_meta_key_import_id() { + return self::META_KEY_IMPORT_ID; + } + /** * Get the optional fields in the schema. * @@ -84,7 +107,7 @@ public function get_required_fields() { return array_values( array_filter( array_map( - function( $field ) use ( $schema ) { + function ( $field ) use ( $schema ) { if ( ! empty( $schema[ $field ]['required'] ) ) { return $field; } @@ -108,7 +131,7 @@ public function get_optional_fields() { return array_values( array_filter( array_map( - function( $field ) use ( $schema ) { + function ( $field ) use ( $schema ) { if ( empty( $schema[ $field ]['required'] ) ) { return $field; } @@ -135,7 +158,7 @@ protected function get_allowed_mime_types( $filter_type = null ) { return array_filter( get_allowed_mime_types(), - function( $mime_type ) use ( $filter_type ) { + function ( $mime_type ) use ( $filter_type ) { return 0 === strpos( $mime_type, $filter_type ); } ); diff --git a/includes/data-port/models/class-sensei-import-model.php b/includes/data-port/models/class-sensei-import-model.php index 9baa471011..fc942f5946 100644 --- a/includes/data-port/models/class-sensei-import-model.php +++ b/includes/data-port/models/class-sensei-import-model.php @@ -106,28 +106,93 @@ public static function from_source_array( $line_number, $data, Sensei_Data_Port_ /** * Check to see if the post already exists in the database. * - * @return int + * Lookup precedence: + * 1. Slug (`post_name`). + * 2. Job-local map (`Sensei_Import_Job::get_import_id()`). + * 3. Source-import-id post meta (`_sensei_import_id`) — durable across runs. + * + * Rows with no slug and no source `id` cannot be matched durably and will + * be re-inserted on re-import. + * + * @since $$next-version$$ + * + * @return int|null */ protected function get_existing_post_id() { - $post_id = null; - $data = $this->get_data(); + $data = $this->get_data(); - if ( ! empty( $data[ $this->schema->get_column_slug() ] ) ) { + // 1. Slug fast path — unchanged behavior. + $slug = isset( $data[ $this->schema->get_column_slug() ] ) ? (string) $data[ $this->schema->get_column_slug() ] : ''; + if ( '' !== $slug ) { $existing_posts = get_posts( - [ + array( 'post_type' => $this->schema->get_post_type(), - 'post_name__in' => [ $data[ $this->schema->get_column_slug() ] ], + 'post_name__in' => array( $slug ), 'posts_per_page' => 1, 'post_status' => 'any', - ] + 'fields' => 'ids', + ) ); if ( ! empty( $existing_posts[0] ) ) { - return $existing_posts[0]->ID; + return (int) $existing_posts[0]; } } - return $post_id; + // 2 & 3. Resolve by source import id — first via the in-memory job map, + // then via the durable post meta. + $import_id = isset( $data[ $this->schema->get_column_id() ] ) ? (string) $data[ $this->schema->get_column_id() ] : ''; + if ( '' !== $import_id && $this->task ) { + $mapped = $this->task->get_job()->get_import_id( $this->schema->get_post_type(), $import_id ); + if ( $mapped ) { + return (int) $mapped; + } + + $post_id = $this->get_existing_post_id_by_import_id( $import_id ); + if ( $post_id ) { + return $post_id; + } + } + + return null; + } + + /** + * Look up an existing post id by the durable source import id meta. + * + * Scoped to the schema's post type so `_sensei_import_id = 7` on a course + * does not match a lesson with the same id. Trashed and auto-draft posts + * are excluded to match the slug path, where `post_status => 'any'` never + * returns those statuses. + * + * @since $$next-version$$ + * + * @param string $import_id The source import id from the CSV row. + * + * @return int|null + */ + private function get_existing_post_id_by_import_id( $import_id ) { + global $wpdb; + + // phpcs:disable WordPress.DB.DirectDatabaseQuery.DirectQuery, WordPress.DB.DirectDatabaseQuery.NoCaching -- Single indexed postmeta lookup by meta key; no cache layer exists for this path. + $post_id = (int) $wpdb->get_var( + $wpdb->prepare( + "SELECT pm.post_id + FROM {$wpdb->postmeta} pm + INNER JOIN {$wpdb->posts} p ON p.ID = pm.post_id + WHERE pm.meta_key = %s + AND pm.meta_value = %s + AND p.post_type = %s + AND p.post_status NOT IN ( 'trash', 'auto-draft' ) + ORDER BY p.ID ASC + LIMIT 1", + $this->schema->get_meta_key_import_id(), + $import_id, + $this->schema->get_post_type() + ) + ); + + return $post_id > 0 ? $post_id : null; } /** @@ -483,13 +548,35 @@ public function add_warnings_to_job() { } /** - * Stores an import id to the job. + * Stores an import id on the post and the job. + * + * The post meta is the durable identity; the in-memory job map remains the + * fast path within a single run. An existing meta value is never + * overwritten, so a slug-resolved post whose source id has changed keeps + * the id it was first written with. The in-memory map is also not + * overwritten when the post already carries a different id, matching the + * meta-write contract. + * + * @since $$next-version$$ */ protected function store_import_id() { $import_id = $this->get_value( $this->schema->get_column_id() ); - if ( ! empty( $import_id ) && $this->task ) { - $this->task->get_job()->set_import_id( $this->schema->get_post_type(), $import_id, $this->get_post_id() ); + if ( empty( $import_id ) || ! $this->task ) { + return; + } + + $post_id = $this->get_post_id(); + $meta_key = $this->schema->get_meta_key_import_id(); + + $existing = (string) get_post_meta( $post_id, $meta_key, true ); + + if ( '' === $existing ) { + update_post_meta( $post_id, $meta_key, (string) $import_id ); + } + + if ( '' === $existing || (string) $existing === (string) $import_id ) { + $this->task->get_job()->set_import_id( $this->schema->get_post_type(), $import_id, $post_id ); } } diff --git a/tests/unit-tests/data-port/models/test-class-sensei-data-port-lesson-model.php b/tests/unit-tests/data-port/models/test-class-sensei-data-port-lesson-model.php index 78a52a1f0f..2b22befd7c 100644 --- a/tests/unit-tests/data-port/models/test-class-sensei-data-port-lesson-model.php +++ b/tests/unit-tests/data-port/models/test-class-sensei-data-port-lesson-model.php @@ -738,4 +738,162 @@ public function testSyncPost_PrivateStatusGiven_CreatesLessonWithPrivateStatus() $this->assertNotEmpty( $post, 'Private lesson should exist.' ); $this->assertEquals( 'private', $post[0]->post_status, 'Lesson post status should be private.' ); } + + /** + * A slug match resolves the existing post even when the source id is absent. + */ + public function testGetExistingPostId_BySlug_ReturnsExisting() { + $existing_id = $this->factory->lesson->create( array( 'post_name' => 'intro' ) ); + + $task = new Sensei_Import_Lessons( Sensei_Import_Job::create( 'test', 0 ) ); + $data = array( + Sensei_Data_Port_Lesson_Schema::COLUMN_TITLE => 'Intro lesson', + Sensei_Data_Port_Lesson_Schema::COLUMN_SLUG => 'intro', + ); + $model = Sensei_Import_Lesson_Model::from_source_array( 1, $data, new Sensei_Data_Port_Lesson_Schema(), $task ); + + $this->assertSame( $existing_id, $model->get_post_id(), 'Slug match should resolve to the existing post id.' ); + } + + /** + * The slug-matched post is treated as not-new, so sync_post updates it + * rather than inserting a duplicate. + */ + public function testGetExistingPostId_BySlug_MarksPostAsNotNew() { + $this->factory->lesson->create( array( 'post_name' => 'intro' ) ); + + $task = new Sensei_Import_Lessons( Sensei_Import_Job::create( 'test', 0 ) ); + $data = array( + Sensei_Data_Port_Lesson_Schema::COLUMN_TITLE => 'Intro lesson', + Sensei_Data_Port_Lesson_Schema::COLUMN_SLUG => 'intro', + ); + $model = Sensei_Import_Lesson_Model::from_source_array( 1, $data, new Sensei_Data_Port_Lesson_Schema(), $task ); + + $this->assertFalse( $model->is_new(), 'Slug match should be treated as not-new.' ); + } + + /** + * When the slug is empty but the source id matches a stored meta, the post + * is resolved via the durable identity — the +22 case from #8066. + */ + public function testGetExistingPostId_ByIdWithNoSlug_ReturnsExisting() { + $existing_id = $this->factory->lesson->create( array( 'post_name' => '' ) ); + update_post_meta( $existing_id, Sensei_Data_Port_Schema::META_KEY_IMPORT_ID, '42' ); + + $task = new Sensei_Import_Lessons( Sensei_Import_Job::create( 'test', 0 ) ); + $data = array( + Sensei_Data_Port_Lesson_Schema::COLUMN_ID => '42', + Sensei_Data_Port_Lesson_Schema::COLUMN_TITLE => 'No slug lesson', + ); + $model = Sensei_Import_Lesson_Model::from_source_array( 1, $data, new Sensei_Data_Port_Lesson_Schema(), $task ); + + $this->assertSame( $existing_id, $model->get_post_id(), 'Meta id match should resolve to the existing post id.' ); + } + + /** + * The id meta written by an earlier run resolves a new run even when the + * in-memory job map is empty. + */ + public function testGetExistingPostId_IdMatchesPostInAnotherJob_ReturnsThatPost() { + $first_task = new Sensei_Import_Lessons( Sensei_Import_Job::create( 'first', 0 ) ); + $first_data = array( + Sensei_Data_Port_Lesson_Schema::COLUMN_ID => '99', + Sensei_Data_Port_Lesson_Schema::COLUMN_TITLE => 'Carried over', + ); + $first_model = Sensei_Import_Lesson_Model::from_source_array( 1, $first_data, new Sensei_Data_Port_Lesson_Schema(), $first_task ); + + $first_model->sync_post(); + + $second_task = new Sensei_Import_Lessons( Sensei_Import_Job::create( 'second', 0 ) ); + $second_model = Sensei_Import_Lesson_Model::from_source_array( 1, $first_data, new Sensei_Data_Port_Lesson_Schema(), $second_task ); + + $existing = get_posts( + array( + 'post_type' => 'lesson', + 'post_status' => 'any', + 's' => 'Carried over', + 'posts_per_page' => -1, + 'fields' => 'ids', + ) + ); + + $this->assertCount( 1, $existing, 'Exactly one lesson should exist after re-import (no duplicate insert).' ); + $this->assertSame( $first_model->get_post_id(), $second_model->get_post_id(), 'Meta written by an earlier run should resolve the post in a new run.' ); + } + + /** + * The id lookup is scoped to the schema's post type so a matching id on a + * different post type does not collide. + */ + public function testGetExistingPostId_IdScopedToPostType_DoesNotMatchOtherType() { + $course_id = $this->factory->course->create(); + update_post_meta( $course_id, Sensei_Data_Port_Schema::META_KEY_IMPORT_ID, '7' ); + + $task = new Sensei_Import_Lessons( Sensei_Import_Job::create( 'test', 0 ) ); + $data = array( + Sensei_Data_Port_Lesson_Schema::COLUMN_ID => '7', + Sensei_Data_Port_Lesson_Schema::COLUMN_TITLE => 'Lesson with course id', + ); + $model = Sensei_Import_Lesson_Model::from_source_array( 1, $data, new Sensei_Data_Port_Lesson_Schema(), $task ); + + $this->assertTrue( $model->is_new(), 'Lesson with an id only present on a course should be treated as new.' ); + } + + /** + * A row with no source id and no slug has no durable identity and is + * re-inserted on re-import. Documented limitation. + */ + public function testGetExistingPostId_NoIdAndNoSlug_CreatesNew() { + $this->factory->lesson->create( + array( + 'post_title' => 'Same title', + 'post_name' => '', + ) + ); + + $task = new Sensei_Import_Lessons( Sensei_Import_Job::create( 'test', 0 ) ); + $data = array( + Sensei_Data_Port_Lesson_Schema::COLUMN_TITLE => 'Same title', + ); + $model = Sensei_Import_Lesson_Model::from_source_array( 1, $data, new Sensei_Data_Port_Lesson_Schema(), $task ); + + $this->assertTrue( $model->is_new(), 'No id and no slug resolves to no existing post; model is new.' ); + $this->assertNull( $model->get_post_id(), 'No id and no slug resolves to no post id before sync_post.' ); + } + + /** + * After a successful import, the source id is written to the post meta. + */ + public function testStoreImportId_WritesMetaAfterInsert() { + $task = new Sensei_Import_Lessons( Sensei_Import_Job::create( 'test', 0 ) ); + $data = array( + Sensei_Data_Port_Lesson_Schema::COLUMN_ID => '500', + Sensei_Data_Port_Lesson_Schema::COLUMN_TITLE => 'Meta write lesson', + ); + $model = Sensei_Import_Lesson_Model::from_source_array( 1, $data, new Sensei_Data_Port_Lesson_Schema(), $task ); + + $model->sync_post(); + + $this->assertSame( '500', (string) get_post_meta( $model->get_post_id(), Sensei_Data_Port_Schema::META_KEY_IMPORT_ID, true ) ); + } + + /** + * An existing meta value is not overwritten by a later import. + */ + public function testStoreImportId_DoesNotOverwriteDifferentId() { + $existing_id = $this->factory->lesson->create( array( 'post_name' => 'kept' ) ); + update_post_meta( $existing_id, Sensei_Data_Port_Schema::META_KEY_IMPORT_ID, 'a' ); + + $task = new Sensei_Import_Lessons( Sensei_Import_Job::create( 'test', 0 ) ); + $data = array( + Sensei_Data_Port_Lesson_Schema::COLUMN_ID => 'b', + Sensei_Data_Port_Lesson_Schema::COLUMN_TITLE => 'Updated', + Sensei_Data_Port_Lesson_Schema::COLUMN_SLUG => 'kept', + ); + $model = Sensei_Import_Lesson_Model::from_source_array( 1, $data, new Sensei_Data_Port_Lesson_Schema(), $task ); + + $model->sync_post(); + + $this->assertSame( 'a', (string) get_post_meta( $existing_id, Sensei_Data_Port_Schema::META_KEY_IMPORT_ID, true ) ); + } }