Skip to content

Commit 7bf60ea

Browse files
pxpmStyleCIBot
pxpm
and
StyleCIBot
authored
Uploaders - Refactor and fixes (#5478)
* add method to get ajax uploaders * Apply fixes from StyleCI [ci skip] [skip ci] * use an abstract class * wip * Apply fixes from StyleCI [ci skip] [skip ci] * refactor uploaders * Apply fixes from StyleCI [ci skip] [skip ci] * refactor rules * Apply fixes from StyleCI [ci skip] [skip ci] * move ajax to PRO, cleanup * Apply fixes from StyleCI [ci skip] [skip ci] * make attributes available for all subfields * fix tests * wip * Apply fixes from StyleCI [ci skip] [skip ci] * upload multiple and upload properly working 🙏 * fixes * Apply fixes from StyleCI [ci skip] [skip ci] * allow the configuration of valueWithoutPath call. * fix valid upload inside repeatables * Apply fixes from StyleCI [ci skip] [skip ci] * fix condition * cleanup * fix * Apply fixes from StyleCI [ci skip] [skip ci] * fix use case for enabling validation after entry is created * Apply fixes from StyleCI [ci skip] [skip ci] * dont save array keys * fix ajax validation * fix validation messages * Apply fixes from StyleCI [ci skip] [skip ci] * fixes ValidUpload * Apply fixes from StyleCI [ci skip] [skip ci] * dont json encode if casted in the model * Apply fixes from StyleCI [ci skip] [skip ci] * fix previous file identification in repeatable * Apply fixes from StyleCI [ci skip] [skip ci] * fix getting values * Apply fixes from StyleCI [ci skip] [skip ci] * add fake fields support * Apply fixes from StyleCI [ci skip] [skip ci] * wip add uploaders tests * Apply fixes from StyleCI [ci skip] [skip ci] * wip * Apply fixes from StyleCI [ci skip] [skip ci] * add pro columns * fix test suite * fix tests * ffix tests * remove unused test views * add uploaders to test coverage * Apply fixes from StyleCI [ci skip] [skip ci] * add coverage folder to gitignore * make tests run faster by not reloading db when not necessary * add coverage to validation tests * add fake tests to uploaders * Apply fixes from StyleCI [ci skip] [skip ci] * add more tests * Apply fixes from StyleCI [ci skip] [skip ci] * wip * Apply fixes from StyleCI [ci skip] [skip ci] * wip * wip * Apply fixes from StyleCI [ci skip] [skip ci] * wip * Apply fixes from StyleCI [ci skip] [skip ci] * add more upload assets * fixes * Apply fixes from StyleCI [ci skip] [skip ci] * fix single file * Apply fixes from StyleCI [ci skip] [skip ci] * add image column * fix tests * Apply fixes from StyleCI [ci skip] [skip ci] * remove hardcoded macro names * Apply fixes from StyleCI [ci skip] [skip ci] * remove double loop, fix single file uploader * Apply fixes from StyleCI [ci skip] [skip ci] * use a big increments and unsigned for primary key * handle pivot file deletion * Apply fixes from StyleCI [ci skip] [skip ci] * register events for relation models * Apply fixes from StyleCI [ci skip] [skip ci] * fix typo * Apply fixes from StyleCI [ci skip] [skip ci] * fix relationship uploaders * Apply fixes from StyleCI [ci skip] [skip ci] * wip * Apply fixes from StyleCI [ci skip] [skip ci] * wip * Apply fixes from StyleCI [ci skip] [skip ci] * update temporary time key * save objects in the macro --------- Co-authored-by: StyleCI Bot <[email protected]>
1 parent 9ed23aa commit 7bf60ea

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

61 files changed

+2393
-279
lines changed

‎.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,5 @@ composer.lock
77
.phpunit.result.cache
88
src/public/packages/
99
/.phpunit.cache
10+
coverage/
1011

‎composer.json

+5-2
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,12 @@
6262
]
6363
},
6464
"scripts": {
65-
"test": "vendor/bin/phpunit --testdox",
65+
"test": [
66+
"@putenv XDEBUG_MODE=off",
67+
"vendor/bin/phpunit"
68+
],
6669
"test-failing": "vendor/bin/phpunit --order-by=defects --stop-on-failure",
67-
"test-coverage": "XDEBUG_MODE=coverage vendor/bin/phpunit --coverage-text"
70+
"test-coverage": "XDEBUG_MODE=coverage vendor/bin/phpunit --coverage-html=coverage"
6871
},
6972
"extra": {
7073
"branch-alias": {

‎phpunit.xml

+3
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
<include>
2424
<directory suffix=".php">./src/app/Library/CrudPanel/Traits/</directory>
2525
<directory>./src/app/Library/Validation/</directory>
26+
<directory>./src/app/Library/Uploaders/</directory>
2627
<directory suffix=".php">./src/app/Library/CrudPanel/</directory>
2728
<directory suffix=".php">./src/app/Models/Traits/</directory>
2829
<file>./src/app/Library/Widget.php</file>
@@ -35,9 +36,11 @@
3536
</source>
3637
<php>
3738
<env name="APP_ENV" value="testing"/>
39+
<env name="APP_KEY" value="AckfSECXIvnK5r28GVIWUAxmbBSjTsmF"/>
3840
<env name="BCRYPT_ROUNDS" value="12"/>
3941
<env name="CACHE_DRIVER" value="array"/>
4042
<env name="DB_CONNECTION" value="sqlite"/>
43+
<env name="DB_FOREIGN_KEYS" value="true"/>
4144
<env name="DB_DATABASE" value=":memory:"/>
4245
<env name="MAIL_MAILER" value="array"/>
4346
<env name="QUEUE_CONNECTION" value="sync"/>

‎src/app/Library/CrudPanel/Traits/FieldsProtectedMethods.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,8 @@ protected function makeSureSubfieldsHaveNecessaryAttributes($field)
281281
$subfield['name'] = Str::replace(' ', '', $subfield['name']);
282282

283283
$subfield['parentFieldName'] = $field['name'];
284+
$subfield['baseFieldName'] = is_array($subfield['name']) ? implode(',', $subfield['name']) : $subfield['name'];
285+
$subfield['baseFieldName'] = Str::afterLast($subfield['baseFieldName'], '.');
284286

285287
if (! isset($field['model'])) {
286288
// we're inside a simple 'repeatable' with no model/relationship, so
@@ -294,8 +296,6 @@ protected function makeSureSubfieldsHaveNecessaryAttributes($field)
294296
$currentEntity = $subfield['baseEntity'] ?? $field['entity'];
295297
$subfield['baseModel'] = $subfield['baseModel'] ?? $field['model'];
296298
$subfield['baseEntity'] = isset($field['baseEntity']) ? $field['baseEntity'].'.'.$currentEntity : $currentEntity;
297-
$subfield['baseFieldName'] = is_array($subfield['name']) ? implode(',', $subfield['name']) : $subfield['name'];
298-
$subfield['baseFieldName'] = Str::afterLast($subfield['baseFieldName'], '.');
299299
}
300300

301301
$field['subfields'][$key] = $this->makeSureFieldHasNecessaryAttributes($subfield);

‎src/app/Library/Uploaders/MultipleFiles.php

+25-3
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,16 @@ public function uploadFiles(Model $entry, $value = null)
5757
}
5858
}
5959

60-
return isset($entry->getCasts()[$this->getName()]) ? $previousFiles : json_encode($previousFiles);
60+
$previousFiles = array_values($previousFiles);
61+
62+
if (empty($previousFiles)) {
63+
return null;
64+
}
65+
66+
return isset($entry->getCasts()[$this->getName()]) || $this->isFake() ? $previousFiles : json_encode($previousFiles);
6167
}
6268

69+
/** @codeCoverageIgnore */
6370
public function uploadRepeatableFiles($files, $previousRepeatableValues, $entry = null)
6471
{
6572
$fileOrder = $this->getFileOrderFromRequest();
@@ -73,11 +80,26 @@ public function uploadRepeatableFiles($files, $previousRepeatableValues, $entry
7380
}
7481
}
7582
}
83+
// create a temporary variable that we can unset keys
84+
// everytime one is found. That way we avoid iterating
85+
// already handled keys (notice we do a deep array copy)
86+
$tempFileOrder = array_map(function ($item) {
87+
return $item;
88+
}, $fileOrder);
7689

7790
foreach ($previousRepeatableValues as $previousRow => $previousFiles) {
7891
foreach ($previousFiles ?? [] as $key => $file) {
79-
$key = array_search($file, $fileOrder, true);
80-
if ($key === false) {
92+
$previousFileInArray = array_filter($tempFileOrder, function ($items, $key) use ($file, $tempFileOrder) {
93+
$found = array_search($file, $items ?? [], true);
94+
if ($found !== false) {
95+
Arr::forget($tempFileOrder, $key.'.'.$found);
96+
97+
return true;
98+
}
99+
100+
return false;
101+
}, ARRAY_FILTER_USE_BOTH);
102+
if ($file && ! $previousFileInArray) {
81103
Storage::disk($this->getDisk())->delete($file);
82104
}
83105
}

‎src/app/Library/Uploaders/SingleBase64Image.php

+3-2
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use Illuminate\Support\Facades\Storage;
88
use Illuminate\Support\Str;
99

10+
/** @codeCoverageIgnore */
1011
class SingleBase64Image extends Uploader
1112
{
1213
public function uploadFiles(Model $entry, $value = null)
@@ -51,7 +52,7 @@ public function uploadRepeatableFiles($values, $previousRepeatableValues, $entry
5152
}
5253
}
5354

54-
$imagesToDelete = array_diff($previousRepeatableValues, $values);
55+
$imagesToDelete = array_diff(array_filter($previousRepeatableValues), $values);
5556

5657
foreach ($imagesToDelete as $image) {
5758
Storage::disk($this->getDisk())->delete($image);
@@ -65,7 +66,7 @@ protected function shouldUploadFiles($value): bool
6566
return $value && is_string($value) && Str::startsWith($value, 'data:image');
6667
}
6768

68-
protected function shouldKeepPreviousValueUnchanged(Model $entry, $entryValue): bool
69+
public function shouldKeepPreviousValueUnchanged(Model $entry, $entryValue): bool
6970
{
7071
return $entry->exists && is_string($entryValue) && ! Str::startsWith($entryValue, 'data:image');
7172
}

‎src/app/Library/Uploaders/SingleFile.php

+9-4
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ public function uploadFiles(Model $entry, $value = null)
3838
return $previousFile;
3939
}
4040

41+
/** @codeCoverageIgnore */
4142
public function uploadRepeatableFiles($values, $previousRepeatableValues, $entry = null)
4243
{
4344
$orderedFiles = $this->getFileOrderFromRequest();
@@ -53,9 +54,13 @@ public function uploadRepeatableFiles($values, $previousRepeatableValues, $entry
5354
}
5455

5556
foreach ($previousRepeatableValues as $row => $file) {
56-
if ($file && ! isset($orderedFiles[$row])) {
57-
$orderedFiles[$row] = null;
58-
Storage::disk($this->getDisk())->delete($file);
57+
if ($file) {
58+
if (! isset($orderedFiles[$row])) {
59+
$orderedFiles[$row] = null;
60+
}
61+
if (! in_array($file, $orderedFiles)) {
62+
Storage::disk($this->getDisk())->delete($file);
63+
}
5964
}
6065
}
6166

@@ -65,7 +70,7 @@ public function uploadRepeatableFiles($values, $previousRepeatableValues, $entry
6570
/**
6671
* Single file uploaders send no value when they are not dirty.
6772
*/
68-
protected function shouldKeepPreviousValueUnchanged(Model $entry, $entryValue): bool
73+
public function shouldKeepPreviousValueUnchanged(Model $entry, $entryValue): bool
6974
{
7075
return is_string($entryValue);
7176
}

‎src/app/Library/Uploaders/Support/Interfaces/UploaderInterface.php

+8
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ public function repeats(string $repeatableContainerName): self;
3030

3131
public function relationship(bool $isRelation): self;
3232

33+
public function fake(bool|string $isFake): self;
34+
3335
/**
3436
* Getters.
3537
*/
@@ -60,4 +62,10 @@ public function canHandleMultipleFiles(): bool;
6062
public function isRelationship(): bool;
6163

6264
public function getPreviousFiles(Model $entry): mixed;
65+
66+
public function getValueWithoutPath(?string $value = null): ?string;
67+
68+
public function isFake(): bool;
69+
70+
public function getFakeAttribute(): bool|string;
6371
}

‎src/app/Library/Uploaders/Support/RegisterUploadEvents.php

+15-2
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ private function registerSubfieldEvent(array $subfield, bool $registerModelEvent
6060
$uploader = $this->getUploader($subfield, $this->uploaderConfiguration);
6161
$crudObject = $this->crudObject->getAttributes();
6262
$uploader = $uploader->repeats($crudObject['name']);
63+
$uploader = $uploader->fake((isset($crudObject['fake']) && $crudObject['fake']) ? ($crudObject['store_in'] ?? 'extras') : false);
6364

6465
// If this uploader is already registered bail out. We may endup here multiple times when doing modifications to the crud object.
6566
// Changing `subfields` properties will call the macros again. We prevent duplicate entries by checking
@@ -139,6 +140,14 @@ private function setupModelEvents(string $model, UploaderInterface $uploader): v
139140
$uploader->deleteUploadedFiles($entry);
140141
});
141142

143+
// if the uploader is a relationship and handles repeatable files, we will also register the deleting event on the
144+
// parent model. that way we can control the deletion of the files when the parent model is deleted.
145+
if ($uploader->isRelationship() && $uploader->handleRepeatableFiles) {
146+
app('crud')->model::deleting(function ($entry) use ($uploader) {
147+
$uploader->deleteUploadedFiles($entry);
148+
});
149+
}
150+
142151
app('UploadersRepository')->markAsHandled($uploader->getIdentifier());
143152
}
144153

@@ -154,9 +163,13 @@ private function setupModelEvents(string $model, UploaderInterface $uploader): v
154163
*/
155164
private function getUploader(array $crudObject, array $uploaderConfiguration): UploaderInterface
156165
{
157-
$customUploader = isset($uploaderConfiguration['uploader']) && class_exists($uploaderConfiguration['uploader']);
166+
$hasCustomUploader = isset($uploaderConfiguration['uploader']);
167+
168+
if ($hasCustomUploader && ! is_a($uploaderConfiguration['uploader'], UploaderInterface::class, true)) {
169+
throw new Exception('Invalid uploader class provided for '.$this->crudObjectType.' type: '.$crudObject['type']);
170+
}
158171

159-
if ($customUploader) {
172+
if ($hasCustomUploader) {
160173
return $uploaderConfiguration['uploader']::for($crudObject, $uploaderConfiguration);
161174
}
162175

0 commit comments

Comments
 (0)