First implementation of the extra fields management structure by Jeremie-Kiwik · Pull Request #41090 · PrestaShop/PrestaShop

Jeremie-Kiwik · 2026-03-25T15:03:52Z

Questions	Answers
Branch?	develop
Description?	First implementation of the extra fields management structure
Type?	new feature
Category?	CO
BC breaks?	no
Deprecations?	no
How to test?	Test module available here: PrestaShop/example-modules#246
UI Tests
Fixed issue or discussion?	Implemented in #40767
Related PRs
Sponsor company	KIWIK

See the .md diff to review the differences between the proposed architecture and the initial implementation.

ps-jarvis · 2026-03-25T15:04:04Z

This pull request seems to contain new translation strings. I have summarized them below to ease up review:

Modules.Extrafieldproduct.Admin ⚠️
- Video link
- Video URL per language

(Note: this is an automated message, but answering it will reach a real human)

kpodemski · 2026-03-25T16:54:23Z

+  `display_bo` tinyint(1) unsigned NOT NULL DEFAULT 1,
+  `display_grid` tinyint(1) unsigned NOT NULL DEFAULT 0,


what's the difference between display_bo and display_grid?

display_bo is to having the fields in the Forms (so you can edit them). display_grid is when you also want to add them on the BO grid of the object. For example:

display_grid:

display_bo:

ok, display_bo is a bit vague, don't you think? I know it's a nitpick, but since it's early and we need to think carefuly about the implementation because it will be hard to change it in the future, I'm sharing my feedback

display_form
display_listing
display_api

This is a bit clearer to me. And will the tab you showed in the screenshot be automatically added whenever we have a custom field? I think that won't be the case, right? Because only the product has such capabilities.

I admit that I haven't read all the specifications just yet, I'm sorry, it's probably faster for me to just ask you, I hope you don't mind 🙏🏻

No problem, it’s quite a massive commit to go through 🙂

For the names, you're right. If you have questions on your first read, then it means it is confusing and needs to be consolidated ^^

So we would have something like this?

display_front: display in FO

display_listing: display in BO grid

display_form: display in BO form

display_api: display in API

listing_position: choose position for BO grid

form_position: choose position for BO form

Regarding form positioning, it works for both types of forms, whether they have tabs or not.

In case of tabbed forms:

by default, it adds an "Extra fields" tab and places the fields inside it

however, you can choose to place fields in another tab using property_path

the "Extra fields" tab is only added if there are actually fields to display

To detect whether a form is tabbed, I traverse the form type hierarchy using getInnerType() and check whether one of the parent types is a NavigationTabType.

kpodemski · 2026-03-25T16:55:07Z

+  `display_api` tinyint(1) unsigned NOT NULL DEFAULT 0,
+  `display_bo` tinyint(1) unsigned NOT NULL DEFAULT 1,
+  `display_grid` tinyint(1) unsigned NOT NULL DEFAULT 0,
+  `grid_position` int(10) unsigned DEFAULT NULL,


That one's tricky, I think, would it be computed automatically somehow?

This makes it easy to quickly filter the scopes in which fields are visible, and helps separate responsibilities. But you're right, it does result in quite a few columns in the database 😅:

FO

display_front to display

BO Grid

display_grid to display

grid_position to define its position (optional)

BO Form

display_bo to display/edit

property_path to define its position (optional)

Admin API

display_api to display/edit

So we can absolutely imagine a different approach.
This is just a first version, with the advantage of being simple and working without introducing additional complexity 🙂. I'm open to every idea

@kpodemski I think you are confusing this "position" thing with the principle of ordering elements in a grid right? In which case the "position" field is the one containing the order indexed

Actually here it's the position of the fields inside the columns, based on which columns are already present. Do you see the idea?
For example grid_position = name -> my field will be added after the name column

I understood the naming because I was aware of the feature, but I understand why it's confusing from your point of view Sadaly I don't have a better alternative naming to match what we aim for 🤔 Do you have some?

jolelievre · 2026-04-09T08:05:38Z

-| **DB table suffix** | `_extra`, `_extra_lang`, `_extra_shop` |
-| **Column naming** | `{module_name}_{field_name}` |
-| **Column name max length** | 64 characters (MariaDB identifier limit) |
+


No diff here (except blank linces), should be reverted

jolelievre · 2026-04-09T08:05:56Z

-| `Common` | `{entity}_extra` | Same value across all shops and languages |
-| `Lang` | `{entity}_extra_lang` | Value varies per language (and per shop if multilang_shop) |
-| `Shop` | `{entity}_extra_shop` | Value varies per shop |
+


jolelievre · 2026-04-09T08:06:56Z

  `id_extra_property_definition` int(10) unsigned NOT NULL AUTO_INCREMENT,
  `entity_name` varchar(64) NOT NULL,
-  `module_name` varchar(64) NOT NULL,
+  `module_name` varchar(64) NOT NULL DEFAULT '',


Is the empty value for core properties? If so, I think we should use null instead of an empty string to identify the core values

jolelievre · 2026-04-09T08:09:49Z

+  `validator` varchar(64) DEFAULT NULL,
  `choices` text DEFAULT NULL,
-  `api_visible` tinyint(1) unsigned NOT NULL DEFAULT 1,
+  `display_front` tinyint(1) unsigned NOT NULL DEFAULT 0,


I'm not sure what display_front is used for? I thought the extra fields were added by modules via hooks or template overrides

If display_front is true does it mean the field is displayed automatically in the FO? Where/how would that be?

We will see later for this filtering feature that raises a lot of questions

jolelievre · 2026-04-09T08:12:13Z

-  `default_value` varchar(255) DEFAULT NULL,
-  `size` int(10) unsigned DEFAULT NULL,
-  `validate` varchar(64) DEFAULT NULL,
+  `storage_column_name` varchar(64) NOT NULL,


Why the extra storage prefix? 🤔 I find it confusing, I'm not even sure to understand what this column is supposed to contain anymore? Is it the DB column name in the extra table?

Isn't it redundant with the field_name ? I though column name were automatically computed with module_name + _ + field_name if this is always the same convention then we don't need to store this info, it can be computed all the times with a simple function

jolelievre · 2026-04-09T12:04:02Z

-### 3.6. ExtraPropertySchemaManager
+> **[impl]** Registry methods return raw associative arrays (not `ExtraPropertyDefinitionCollection` / `ExtraPropertyDefinition` VOs). Three interfaces exist for different read granularities:
+> - `EntityExtraFieldRegistryInterface` — public-facing facade (register/unregister + `getByEntityNameAllScopes()`)
+> - `ExtraPropertyRegistryInterface` — full write + read interface (extends the above)


I'm not sure it's a good idea to mix read and write, maybe a dedicated service should handle the write operations

We should split, at least the interfaces, have the read and the write separated

jolelievre · 2026-04-09T12:05:08Z

+
+> **[impl]** The original proposal returned a flat `['column_name' => value]` map. The actual implementation returns values **grouped by module**: `['_core' => ['field' => value], 'mymodule' => ['field' => value]]`. The method also accepts `primaryKeyName` (not assumed from entity name), `isLangMultishop`, `displayFrontOnly`, and `preloadedDefinitionRows` (to avoid double DB reads when definitions are already loaded).
+
+**Performance**: reads definitions from the registry (cached); if no definitions exist for the entity, returns an empty array immediately without DB query.


So we drop the bulk read for grids?

Ok handled differently via the query builder, b etter approach

jolelievre · 2026-04-09T12:06:15Z

+/**
+ * Bulk write: entity values + all lang rows + shop row in one call.
+ */
+public function writeAll(


Good call to add something to write them all, especially for the API I guess, which will be mostly automated (not handled via a hook)
But it will probably be used for other use cases as well

jolelievre · 2026-04-09T12:07:21Z

+
+- `getByEntityNameAllScopes(string $entityName): array` — returns raw rows for all scopes
+- `findDefinitionByModuleAndField(string $entity, string $module, string $field, string $scope): ?array`
+- `save(array $definitionData): int`


Why a flat array and not a dedicated object?

jolelievre · 2026-04-09T12:08:09Z

-  # Registry: base implementation + cache decorator
-  PrestaShop\PrestaShop\Core\ExtraProperty\Registry\ExtraPropertyRegistry:
+  # Registry: base + cache decorator
+  PrestaShop\PrestaShop\Adapter\ExtraProperty\ExtraPropertyRegistry:


Why put all this in Adapter? It seems legitimate to have it in the Core no?

jolelievre · 2026-04-09T12:46:55Z

+  `field_type` ENUM('int','bool','string','float','date','html','json','choice') NOT NULL,
+  `field_scope` ENUM('common','lang','shop') NOT NULL DEFAULT 'common',
+  `symfony_field_type` varchar(255) DEFAULT NULL,
+  `property_path` varchar(255) DEFAULT NULL,


Suggested change

`property_path` varchar(255) DEFAULT NULL,

`form_position` varchar(255) DEFAULT NULL,

jolelievre · 2026-04-09T12:47:07Z

-  `default_value` varchar(255) DEFAULT NULL,
-  `size` int(10) unsigned DEFAULT NULL,
-  `validate` varchar(64) DEFAULT NULL,
+  `storage_column_name` varchar(64) NOT NULL,


Suggested change

`storage_column_name` varchar(64) NOT NULL,

jolelievre · 2026-04-09T12:47:20Z

-  `size` int(10) unsigned DEFAULT NULL,
-  `validate` varchar(64) DEFAULT NULL,
+  `storage_column_name` varchar(64) NOT NULL,
+  `field_type` ENUM('int','bool','string','float','date','html','json','choice') NOT NULL,


Suggested change

`field_type` ENUM('int','bool','string','float','date','html','json','choice') NOT NULL,

`type` ENUM('int','bool','string','float','date','html','json','choice') NOT NULL,

jolelievre · 2026-04-09T12:47:30Z

-  `validate` varchar(64) DEFAULT NULL,
+  `storage_column_name` varchar(64) NOT NULL,
+  `field_type` ENUM('int','bool','string','float','date','html','json','choice') NOT NULL,
+  `field_scope` ENUM('common','lang','shop') NOT NULL DEFAULT 'common',


Suggested change

`field_scope` ENUM('common','lang','shop') NOT NULL DEFAULT 'common',

`scope` ENUM('common','lang','shop') NOT NULL DEFAULT 'common',

jolelievre · 2026-04-09T12:47:52Z

+  `storage_column_name` varchar(64) NOT NULL,
+  `field_type` ENUM('int','bool','string','float','date','html','json','choice') NOT NULL,
+  `field_scope` ENUM('common','lang','shop') NOT NULL DEFAULT 'common',
+  `symfony_field_type` varchar(255) DEFAULT NULL,


Suggested change

`symfony_field_type` varchar(255) DEFAULT NULL,

`form_field_type` varchar(255) DEFAULT NULL,

jolelievre · 2026-04-09T13:25:47Z

 ```php
 namespace PrestaShop\PrestaShop\Core\ExtraProperty;

-class ExtraPropertyDefinition


This object should not be removed, it will be the content of the ExtraPropertyDefinitionCollection, it will be cleanr to have this object instead of a flatten array

jolelievre · 2026-04-09T13:29:27Z

-**Cache decorator**: `CachedExtraPropertyRegistry` decorates `ExtraPropertyRegistryInterface`, using Symfony's `cache.app` pool to cache results. It exposes an additional `invalidateCache(): void` method (not part of the interface) for cache invalidation.
-
-### 3.6. ExtraPropertySchemaManager
+> **[impl]** Registry methods return raw associative arrays (not `ExtraPropertyDefinitionCollection` / `ExtraPropertyDefinition` VOs). Three interfaces exist for different read granularities:


As we said this should contain a ExtraPropertyDefinition object

jolelievre · 2026-04-09T13:33:19Z

-### 3.6. ExtraPropertySchemaManager
+> **[impl]** Registry methods return raw associative arrays (not `ExtraPropertyDefinitionCollection` / `ExtraPropertyDefinition` VOs). Three interfaces exist for different read granularities:
+> - `EntityExtraFieldRegistryInterface` — public-facing facade (register/unregister + `getByEntityNameAllScopes()`)
+> - `ExtraPropertyRegistryInterface` — full write + read interface (extends the above)


We should split, at least the interfaces, have the read and the write separated

jolelievre · 2026-04-09T13:36:55Z

+
+> **[impl]** The original proposal returned a flat `['column_name' => value]` map. The actual implementation returns values **grouped by module**: `['_core' => ['field' => value], 'mymodule' => ['field' => value]]`. The method also accepts `primaryKeyName` (not assumed from entity name), `isLangMultishop`, `displayFrontOnly`, and `preloadedDefinitionRows` (to avoid double DB reads when definitions are already loaded).
+
+**Performance**: reads definitions from the registry (cached); if no definitions exist for the entity, returns an empty array immediately without DB query.


Ok handled differently via the query builder, b etter approach

Jeremie-Kiwik · 2026-04-24T07:46:41Z

I close this-one. Duplicate of #41092
(I modified ARCHITECTURE.md directly in 41092 -- no need of another PR for that)

Implemented 1st version

9b67ea6

Jeremie-Kiwik requested a review from a team as a code owner March 25, 2026 15:03

github-project-automation Bot added this to PR Dashboard Mar 25, 2026

ps-jarvis added develop Branch Feature Type: New Feature labels Mar 25, 2026

github-project-automation Bot moved this to Ready for review in PR Dashboard Mar 25, 2026

Jeremie-Kiwik mentioned this pull request Mar 25, 2026

Add extra fields to PrestaShop #41092

Open

kpodemski changed the title ~~Implemented 1st version~~ First implementation of the extra fields management structure Mar 25, 2026

kpodemski added this to the 9.2.0 milestone Mar 25, 2026

kpodemski reviewed Mar 25, 2026

View reviewed changes

jolelievre reviewed Apr 9, 2026

View reviewed changes

Jeremie-Kiwik closed this Apr 24, 2026

github-project-automation Bot moved this from Ready for review to Closed in PR Dashboard Apr 24, 2026

cnavarro-prestashop mentioned this pull request May 6, 2026

[Extra properties] Stabilize and merge PR #41092 #41423

Open

		`display_bo` tinyint(1) unsigned NOT NULL DEFAULT 1,
		`display_grid` tinyint(1) unsigned NOT NULL DEFAULT 0,


		> [impl] The original proposal returned a flat `['column_name' => value]` map. The actual implementation returns values grouped by module: `['_core' => ['field' => value], 'mymodule' => ['field' => value]]`. The method also accepts `primaryKeyName` (not assumed from entity name), `isLangMultishop`, `displayFrontOnly`, and `preloadedDefinitionRows` (to avoid double DB reads when definitions are already loaded).

		Performance: reads definitions from the registry (cached); if no definitions exist for the entity, returns an empty array immediately without DB query.

	`property_path` varchar(255) DEFAULT NULL,
	`form_position` varchar(255) DEFAULT NULL,

	`field_type` ENUM('int','bool','string','float','date','html','json','choice') NOT NULL,
	`type` ENUM('int','bool','string','float','date','html','json','choice') NOT NULL,

	`field_scope` ENUM('common','lang','shop') NOT NULL DEFAULT 'common',
	`scope` ENUM('common','lang','shop') NOT NULL DEFAULT 'common',

	`symfony_field_type` varchar(255) DEFAULT NULL,
	`form_field_type` varchar(255) DEFAULT NULL,

Conversation

Jeremie-Kiwik commented Mar 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ps-jarvis commented Mar 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Jeremie-Kiwik commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Jeremie-Kiwik commented Mar 25, 2026 •

edited

Loading