diff --git a/app/Http/Controllers/TableController.php b/app/Http/Controllers/TableController.php index e261007..8b14c47 100644 --- a/app/Http/Controllers/TableController.php +++ b/app/Http/Controllers/TableController.php @@ -213,7 +213,7 @@ class TableController extends Controller $rowsToInsert = null; $rowNumerator = null; try { - $rowsToInsert = Changeset::csvToRowsArray($columns, $dataCsvLines, true)->all(); + $rowsToInsert = (new Changeset)->csvToRowsArray($columns, $dataCsvLines, true, false)->all(); } catch (\Exception $e) { return $this->backWithErrors(['data' => $e->getMessage()]); } diff --git a/app/Http/Controllers/TableEditController.php b/app/Http/Controllers/TableEditController.php index 6a09a6d..a567268 100644 --- a/app/Http/Controllers/TableEditController.php +++ b/app/Http/Controllers/TableEditController.php @@ -334,6 +334,23 @@ class TableEditController extends Controller */ public function draftSubmit(Request $request, User $user, string $table) { + /** @var Table $tableModel */ + $tableModel = $user->tables()->where('name', $table)->first(); + if ($tableModel === null) abort(404, "No such table."); + + $input = $this->validate($request, [ + 'note' => 'required|string' + ]); + + $changeset = $this->getChangeset($tableModel); + $changeset->note = trim($input->note); + if (empty($changeset->note)) throw new SimpleValidationException('note', 'Note is required.'); + + if (!$changeset->getRowChangeCounts()->any && !$changeset->getColumnChangeCounts()->any) { + flash()->error("There are no changes to submit."); + return back(); + } + // } } diff --git a/app/Models/Proposal.php b/app/Models/Proposal.php index a56f313..d3f4db1 100644 --- a/app/Models/Proposal.php +++ b/app/Models/Proposal.php @@ -79,7 +79,7 @@ class Proposal extends BaseModel throw new NotApplicableException('No changes to propose.'); } - if ($changeset->note == null) { + if (empty($changeset->note)) { throw new NotApplicableException('Proposal note must be filled.'); } @@ -91,6 +91,9 @@ class Proposal extends BaseModel throw new NotApplicableException('Revision not assigned to Changeset'); } + // Assign unique row IDs to new rows (they use temporary negative IDs in a draft) + $changeset->renumberRows(); + return new Proposal([ // relations 'table_id' => $changeset->table->getKey(), diff --git a/app/Tables/BaseNumerator.php b/app/Tables/BaseNumerator.php index e94cb41..8d94749 100644 --- a/app/Tables/BaseNumerator.php +++ b/app/Tables/BaseNumerator.php @@ -3,7 +3,9 @@ namespace App\Tables; - +/** + * Sequential ID assigner + */ abstract class BaseNumerator { /** @var int */ @@ -39,10 +41,18 @@ abstract class BaseNumerator throw new \OutOfBoundsException("Column numerator has run out of allocated GCID slots"); $key = $this->getKey($this->next); - $this->next++; + $this->advance(); return $key; } + /** + * Advance to the next ID + */ + protected function advance() + { + $this->next++; + } + /** * Convert numeric index to a key * diff --git a/app/Tables/Changeset.php b/app/Tables/Changeset.php index d68dedd..a5d9225 100644 --- a/app/Tables/Changeset.php +++ b/app/Tables/Changeset.php @@ -104,6 +104,14 @@ class Changeset */ public $removedColumns = []; + /** + * Draft row numerator state holder; numbers grow to negative, + * and are replaced with real unique row IDs when the proposal is submitted. + * + * @var int + */ + public $nextRowID = -1; + /** * Generator iterating all properties, used internally for serialization to array * @@ -160,22 +168,6 @@ class Changeset return $object; } - /** - * Check if there is any change in this changeset - * - * @return bool - any found - */ - public function hasAnyChanges() - { - foreach ($this->walkProps() as $prop) { - if (!empty($this->$prop)) { - return true; - } - } - - return false; - } - /** * Decorate / transform a single row for the editor view * @@ -504,6 +496,9 @@ class Changeset } } + // try creating a column with the new data + new Column(array_merge($col->toArray(), $updateObj)); + if ($this->isNewColumn($id)) { $this->newColumns[$id] = array_merge($this->newColumns[$id], $updateObj); } @@ -569,7 +564,7 @@ class Changeset * so it can be inserted directly into DB * @return Collection */ - public static function csvToRowsArray($columns, $csvArray, $forTableInsert) + public function csvToRowsArray($columns, $csvArray, $forTableInsert, $useDraftIds) { /** @var Collection $rows */ $rows = collect($csvArray)->map(function ($row) use ($columns, $forTableInsert) { @@ -597,9 +592,11 @@ class Changeset } })->filter(); - // TODO we could use some temporary IDs and replace them with - // TODO proper unique IDs when submitting the proposal - $rowNumerator = new RowNumerator(count($csvArray)); + if ($useDraftIds) { + $rowNumerator = new DraftRowNumerator($this, count($csvArray)); + } else { + $rowNumerator = new RowNumerator(count($csvArray)); + } if ($forTableInsert) { return $rows->map(function ($row) use (&$rowNumerator) { @@ -628,9 +625,7 @@ class Changeset $template[$column->id] = $column->cast(null); } - // TODO we could use some temporary IDs and replace them with - // TODO proper unique IDs when submitting the proposal - $numerator = new RowNumerator($count); + $numerator = new DraftRowNumerator($this, $count); foreach ($numerator->generate() as $_id) { $row = $template; @@ -649,7 +644,7 @@ class Changeset /** @var Column[] $columns */ $columns = array_values($this->fetchAndTransformColumns()); - $rows = self::csvToRowsArray($columns, $csvArray, false) + $rows = self::csvToRowsArray($columns, $csvArray, false, true) ->keyBy('_id'); // using '+' to avoid renumbering @@ -803,4 +798,77 @@ class Changeset { $this->rowUpdates = []; } + + /** + * Get row change counts (used for the view) + * + * @return object + */ + public function getRowChangeCounts() + { + $numChangedRows = count($this->rowUpdates); + $numNewRows = count($this->newRows); + $numRemovedRows = count($this->removedRows); + + return (object) [ + 'changed' => $numChangedRows, + 'new' => $numNewRows, + 'removed' => $numRemovedRows, + 'any' => $numChangedRows || $numNewRows || $numRemovedRows, + ]; + } + + /** + * Get column change counts (used for the view) + * + * @return object + */ + public function getColumnChangeCounts() + { + $numChangedColumns = count($this->columnUpdates); + $numNewColumns = count($this->newColumns); + $numRemovedColumns = count($this->removedColumns); + $reordered = count($this->columnOrder) != 0; + + return (object) [ + 'changed' => $numChangedColumns, + 'new' => $numNewColumns, + 'removed' => $numRemovedColumns, + 'reordered' => $reordered, + 'any' => $reordered || $numChangedColumns || $numNewColumns || $numRemovedColumns, + ]; + } + + /** + * Check if there is any change in this changeset + * + * @return bool - any found + */ + public function hasAnyChanges() + { + $colChanges = $this->getColumnChangeCounts(); + $rowChanges = $this->getRowChangeCounts(); + + return $colChanges->any || $rowChanges->any; + } + + /** + * Replace temporary negative row IDs with real unique row IDs + */ + public function renumberRows() + { + $rows = []; + $numerator = new RowNumerator(count($this->newRows)); + foreach ($this->newRows as $row) { + if ($row['_id'] < 0) { + $id = $numerator->next(); + $row['_id'] = $id; + $rows[$id] = $row; + } else { + // keep ID + $rows[$row['_id']] = $row; + } + } + $this->newRows = $rows; + } } diff --git a/app/Tables/Column.php b/app/Tables/Column.php index 80530cd..d2fde28 100644 --- a/app/Tables/Column.php +++ b/app/Tables/Column.php @@ -125,27 +125,27 @@ class Column implements JsonSerializable, Arrayable * * @param object|array $obj */ - public function __construct($obj, $allowEmptyName=false) + public function __construct($obj) { $b = objBag($obj); - if (!$allowEmptyName && (!$b->has('name') || $b->name == '')) { - throw new NotApplicableException('Missing name in column'); + if (!$b->has('name') || $b->name == '') { + throw new SimpleValidationException('name', "Missing column name."); } - if (!$b->has('type')) { - throw new NotApplicableException('Missing type in column'); + if (!$b->has('type') || !in_array($b->type, self::colTypes)) { + throw new SimpleValidationException('type', "Missing or invalid column type."); } - if ($b->name[0] == '_') { // would cause problems with selects (see: _id / GRID) - throw new NotApplicableException("Column name can't start with underscore."); + if ($b->name[0] == '_' || !preg_match(IDENTIFIER_PATTERN, $b->name)) { + throw new SimpleValidationException('name', "Column name can contain only letters, digits, and underscore, and must start with a letter."); } - if (!in_array($b->type, self::colTypes)) { - throw new NotApplicableException("\"$b->type\" is not a valid column type."); + if (!$b->has('id') || $b->id[0] == '_') { + throw new SimpleValidationException('id', "Invalid or missing column ID"); } - $this->id = $b->get('id', null); + $this->id = $b->id; $this->name = $b->name; $this->type = $b->type; $this->title = $b->title ?: $b->name; diff --git a/app/Tables/DraftRowNumerator.php b/app/Tables/DraftRowNumerator.php new file mode 100644 index 0000000..c3ec733 --- /dev/null +++ b/app/Tables/DraftRowNumerator.php @@ -0,0 +1,22 @@ +nextRowID - $capacity + 1, $changeset->nextRowID]); + $changeset->nextRowID -= $capacity; + } +} diff --git a/app/Tables/RowNumerator.php b/app/Tables/RowNumerator.php index c493f1b..80b025e 100644 --- a/app/Tables/RowNumerator.php +++ b/app/Tables/RowNumerator.php @@ -5,6 +5,9 @@ namespace App\Tables; use App\Models\Row; +/** + * Utility for allocating & assigning globally unique row IDs + */ class RowNumerator extends BaseNumerator { /** diff --git a/porklib/helpers.php b/porklib/helpers.php index 77368bb..b3840d7 100644 --- a/porklib/helpers.php +++ b/porklib/helpers.php @@ -16,7 +16,7 @@ use MightyPork\Utils\Utils; #region Defines -define('IDENTIFIER_PATTERN', '/^[a-z][a-z0-9_]*$/i'); +define('IDENTIFIER_PATTERN', '/^[a-z_][a-z0-9_]*$/i'); define('USERNAME_PATTERN', '/^[a-z0-9_-]+$/i'); #endregion diff --git a/resources/assets/js/components/ColumnEditor.vue b/resources/assets/js/components/ColumnEditor.vue index 7907cf5..e95653b 100644 --- a/resources/assets/js/components/ColumnEditor.vue +++ b/resources/assets/js/components/ColumnEditor.vue @@ -61,20 +61,26 @@ Complex animated column editor for the table edit page @@ -262,6 +268,7 @@ export default { }, (resp) => { this.$set(this.columns, n, resp.data) }, (er) => { + console.log("Col save error: ", er) if (!_.isUndefined(er.errors)) { this.$set(this.columns[n], '_errors', er.errors) } diff --git a/resources/views/table/propose/manage-columns.blade.php b/resources/views/table/propose/manage-columns.blade.php index 1e9709d..97ba4cb 100644 --- a/resources/views/table/propose/manage-columns.blade.php +++ b/resources/views/table/propose/manage-columns.blade.php @@ -18,6 +18,7 @@ name: 'columns', route: {!! toJSON($table->draftUpdateRoute) !!}, xColumns: {!! toJSON($columns) !!}, + orderChanged: @json(!empty($changeset->columnOrder)) }) }); diff --git a/resources/views/table/propose/review.blade.php b/resources/views/table/propose/review.blade.php index a07d148..1365e1a 100644 --- a/resources/views/table/propose/review.blade.php +++ b/resources/views/table/propose/review.blade.php @@ -3,20 +3,10 @@ /** @var \App\Tables\Changeset $changeset */ /** @var \App\Models\Table $table */ - $numChangedRows = count($changeset->rowUpdates); - $numNewRows = count($changeset->newRows); - $numRemovedRows = count($changeset->removedRows); + $rowChanges = $changeset->getRowChangeCounts(); + $columnChanges = $changeset->getColumnChangeCounts(); - $anyRowChanges = $numChangedRows || $numNewRows || $numRemovedRows; - - $numChangedColumns = count($changeset->columnUpdates); - $numNewColumns = count($changeset->newColumns); - $numRemovedColumns = count($changeset->removedColumns); - $colsReordered = !empty($changeset->columnOrder); - - $anyColChanges = $numChangedColumns || $numNewColumns || $numRemovedColumns || $colsReordered; - - $anyChanges = ($anyRowChanges || $anyColChanges) && strlen(trim($changeset->note)) > 0; + $anyChanges = ($rowChanges->any || $columnChanges->any) && strlen(trim($changeset->note)) > 0; @endphp @extends('table.propose.layout') @@ -35,27 +25,27 @@ Rows
- @if($anyRowChanges) + @if($rowChanges->any) - @if($numChangedRows) + @if($rowChanges->changed)
- {{ $numChangedRows }} changed + {{ $rowChanges->changed }} changed Reset
@endif - @if($numNewRows) + @if($rowChanges->new)
- {{ $numNewRows }} new + {{ $rowChanges->new }} new Reset
@endif - @if($numRemovedRows) + @if($rowChanges->removed)
- {{ $numRemovedRows }} removed + {{ $rowChanges->removed }} removed Reset
@@ -72,33 +62,33 @@ Columns
- @if($anyColChanges) + @if($columnChanges->any) - @if($numChangedColumns) + @if($columnChanges->changed)
- {{ $numChangedColumns }} changed + {{ $columnChanges->changed }} changed Reset
@endif - @if($numNewColumns) + @if($columnChanges->new)
- {{ $numNewColumns }} new + {{ $columnChanges->new }} new Reset
@endif - @if($numRemovedColumns) + @if($columnChanges->removed)
- {{ $numRemovedColumns }} removed + {{ $columnChanges->removed }} removed Reset
@endif - @if($colsReordered) + @if($columnChanges->reordered)
Order changed - @if($anyRowChanges || $anyColChanges) + @if($rowChanges->any || $columnChanges->any) Write a summary to submit your changes. @else No changes to submit. @@ -148,7 +138,7 @@ ready(function () { app.DraftNotePage({ route: @json($table->draftUpdateRoute), - anyChanges: @json($anyRowChanges || $anyColChanges) + anyChanges: @json($rowChanges->any || $columnChanges->any) }) })