.gitignore (1)

6-6: Root-only ignore for Phpactor config — confirm scope; consider YAML variant

Using a leading slash limits ignoring .phpactor.json to the repo root. If that’s intended, this is fine. If you want to ignore it anywhere in the tree (e.g., in nested packages/fixtures), drop the leading slash. Also, consider whether .phpactor.yml is used in your workflows and should be ignored as well for consistency.

Optional change if you want non-root matching:

-/.phpactor.json
+.phpactor.json

packages/core/src/Facades/StorefrontSession.php (2)

19-24: Add @mixin annotation to StorefrontSession facade

All init*, forget and getSessionKey methods have been confirmed on StorefrontSessionManager. Adding a @mixin will help IDEs infer the full API without needing to enumerate every method.

• File: packages/core/src/Facades/StorefrontSession.php
– In the class-level docblock, add:

 /**
+ * @mixin \Lunar\Managers\StorefrontSessionManager
  * @method static \Lunar\Models\Contracts\Channel getChannel()
  * @method static \Lunar\Managers\StorefrontSessionManager setChannel(\Lunar\Models\Contracts\Channel $channel)
  * @method static \Illuminate\Support\Collection getCustomerGroups()
  * …
  * @method static void initCustomer()
  * @method static void forget()
  * @method static string getSessionKey()

---

11-14: Document concrete Collection value-type for customer groups

We’ve verified that StorefrontSessionManager::__construct() and resetCustomerGroups() always initialize $this->customerGroups to a new Collection, so getCustomerGroups() never returns null. It’s safe to tighten the facade PHPDoc with generics for better IDE/static-analysis support:

File: packages/core/src/Facades/StorefrontSession.php

- * @method static \Illuminate\Support\Collection getCustomerGroups()
+ * @method static \Illuminate\Support\Collection<int, \Lunar\Models\Contracts\CustomerGroup> getCustomerGroups()
- * @method static \Lunar\Managers\StorefrontSessionManager setCustomerGroups(\Illuminate\Support\Collection $customerGroups)
+ * @method static \Lunar\Managers\StorefrontSessionManager setCustomerGroups(\Illuminate\Support\Collection<int, \Lunar\Models\Contracts\CustomerGroup> $customerGroups)

packages/admin/src/Support/Forms/AttributeData.php (2)

66-68: Use the imported contract consistently

Minor consistency: use the FieldTypeContract alias here as well.

-                if ($state instanceof \Lunar\Base\FieldType) {
+                if ($state instanceof FieldTypeContract) {
                     return $state->getValue();
                 }

---

16-16: Nit: fix Dropdown alias typo for clarity

The alias is spelled DrodownFieldType which might be confusing when scanning the file.

-use Lunar\FieldTypes\Dropdown as DrodownFieldType;
+use Lunar\FieldTypes\Dropdown as DropdownFieldType;
@@
-        DrodownFieldType::class => Dropdown::class,
+        DropdownFieldType::class => Dropdown::class,

Also applies to: 31-31

tests/admin/Stubs/FieldTypes/RepeaterField.php (1)

18-35: Optional: be lenient with null/blank inputs in setValue

For test ergonomics, consider treating null/blank as [] instead of throwing, aligning with some core field types’ behavior.

-    public function setValue($value)
+    public function setValue($value)
     {
-        if (! is_array($value)) {
+        if ($value === null) {
+            $value = [];
+        }
+        if (! is_array($value)) {
             throw new FieldTypeException(self::class.' value must be an array.');
         }
 
         $this->value = json_encode($value);
     }

tests/admin/Stubs/Support/FieldTypes/BuilderField.php (1)

24-73: LGTM: UI-layer normalization complements dehydration

The format/dehydrate callbacks mirror the domain stub’s normalize logic, keeping the component state predictable. With the proposed change in AttributeData, we avoid double-filtering of scalar arrays while still reindexing builder items.

If you adopt the AttributeData normalization tweak, please rerun the product edit tests that exercise builder/repeater to confirm no regressions. I can help expand coverage to include File multiple and ListField cases.

tests/admin/Stubs/Support/Synthesizers/BuilderSynth.php (2)

26-38: Avoid double-normalization; let FieldType::setValue() own normalization

You're pre-normalizing in the nested set() path, but the CoreBuilderField::setValue() already normalizes comprehensively (including stripping empty/null data, enforcing shape). Relying on the FieldType normalization avoids duplicate work and keeps all rules in one place.

Suggested change:

     public function set(&$target, $key, $value)
     {
         if ($key === '' || $key === null) {
-            $target->setValue(is_array($value) ? $value : []);
+            // Let the FieldType perform full normalization.
+            $target->setValue(is_array($value) ? $value : []);
 
             return;
         }
 
         $fieldValue = (array) $target->getValue();
         Arr::set($fieldValue, $key, $value);
 
-        $target->setValue($this->normalize($fieldValue));
+        // Delegate normalization to the FieldType implementation.
+        $target->setValue($fieldValue);
     }

---

40-47: Mirror set() simplification in unset()

Same rationale as set(): pre-normalizing here is redundant given CoreBuilderField::setValue() normalizes fully, including re-indexing.

Suggested change:

     public function unset(&$target, $index)
     {
         $fieldValue = (array) $target->getValue();
 
         Arr::forget($fieldValue, $index);
 
-        $target->setValue($this->normalize($fieldValue));
+        $target->setValue($fieldValue);
     }

tests/admin/Feature/Filament/Resources/ProductResource/Pages/EditProductTest.php (4)

78-206: Pre-synthesize here too for consistency and to reduce flakiness

This test registers custom field types (Repeater/Builder). While it only asserts rendering, pre-synthesizing keeps behavior consistent across tests and avoids subtle issues with nested state resolution.

Suggested insertion:

     \Lunar\Admin\Support\Facades\AttributeData::registerFieldType(
         \Lunar\Tests\Admin\Stubs\FieldTypes\RepeaterField::class,
         \Lunar\Tests\Admin\Stubs\Support\FieldTypes\RepeaterField::class,
     );
+    // Preload property synthesizers for dynamic field types
+    \Lunar\Admin\Support\Facades\AttributeData::synthesizeLivewireProperties();

Additionally, since this page renders variant pricing contexts, you already added a default Currency in this test — good. Consider applying the same to the other new tests for consistency (see suggestions below).

---

264-368: Add a default Currency to mirror the other test’s environment

The EditProduct page often expects a default Currency for variant-related UI. You created it in the “render all fields” test but not here. Add it to avoid environment-dependent failures.

Suggested insertion after the TaxClass factory:

     \Lunar\Models\Language::factory()->create(['default' => true]);
     \Lunar\Models\TaxClass::factory()->create(['default' => true]);
+    \Lunar\Models\Currency::factory()->create(['default' => true]);

Optional: there’s a fair bit of setup duplication across tests (language/tax/currency, groups, attributables). If this grows, consider extracting test helpers/factories to keep tests compact.

---

369-486: Pre-synthesize and add default Currency for parity

• Registering the Repeater field type is correct, but, like above, pre-synthesizing reduces surprises in nested array handling.
• Add a default Currency to keep the environment aligned with your other test and the page’s expectations.

Suggested changes:

     \Lunar\Admin\Support\Facades\AttributeData::registerFieldType(
         \Lunar\Tests\Admin\Stubs\FieldTypes\RepeaterField::class,
         \Lunar\Tests\Admin\Stubs\Support\FieldTypes\RepeaterField::class,
     );
+    \Lunar\Admin\Support\Facades\AttributeData::synthesizeLivewireProperties();
     \Lunar\Models\Language::factory()->create([
         'default' => true,
     ]);
 
     \Lunar\Models\TaxClass::factory()->create([
         'default' => true,
     ]);
+    \Lunar\Models\Currency::factory()->create([
+        'default' => true,
+    ]);

Note: Great end-to-end assertion of saved state for both product and variant repeaters.

---

1-486: Add a regression test for custom File field type (covers linked issue #2091)

Given the PR’s objective to let custom file attribute types save correctly (removing strict class checks), please add a test that exercises a custom File subtype end-to-end. This will prevent regressions and explicitly validates the fix for #2091.

I can draft it if helpful. Sketch:

• Define a stub CustomFileField extends \Lunar\FieldTypes\File (no changes).
• Register it with AttributeData to the same Filament file component used by core File.
• Create a product + attribute of type CustomFileField.
• Pre-synthesize, mount EditProduct, fill with new CustomFileField('foo/bar.jpg'), save.
• Assert $product->attr('custom_file') === 'foo/bar.jpg'.

Would you like me to open a follow-up PR with the test and stubs?

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.