Skip to content

Conversation

@brendt
Copy link
Member

@brendt brendt commented Mar 26, 2025

Closes #1074

TODOs:

  • Document: DatabaseModel is gone
  • Document: IsDatabaseModel trait is still available if people want some convenient shorthands like Model::query(), $model->save(), etc. But it's totally optional.
  • Document: Route binding has changed, you now need to implement Bindable
  • Document: rename Model::query() to Model::select()
  • Document: query function
  • The mapper currently assumes there's always an Id field on model classes. This assumption should be made optional, because IDs aren't actually required by the database: Optional ids in ORM #1079
  • Also add QueryModel::delete, QueryModel::insert and QueryModel::create
  • Add functional api on top of QueryModel, perhaps something like query(Book::class)->… ?
  • Add tests for SelectStatement (and also all upcoming statement classes)
  • Add non-model query support
  • Add UpdateStatement, CreateStatement, and DeleteStatement
  • Document: non-model query support Document: non-model query support #1089
  • Introduce model definition helper, something like model($objectOrClass)->hasPrimaryKey(), model($objectOrClass)->primaryKeyName() etc. Improve ModelInspector #1086

@brendt brendt self-assigned this Mar 26, 2025
@brendt brendt added this to the v1.0.0-beta.1 milestone Mar 26, 2025
@brendt brendt marked this pull request as draft March 26, 2025 09:22
@coveralls
Copy link

coveralls commented Mar 26, 2025

Pull Request Test Coverage Report for Build 14122915110

Details

  • 558 of 571 (97.72%) changed or added relevant lines in 42 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.4%) to 79.691%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/Tempest/Database/src/Builder/QueryBuilders/UpdateQueryBuilder.php 53 54 98.15%
src/Tempest/Database/src/IsDatabaseModel.php 41 42 97.62%
src/Tempest/Database/src/Builder/QueryBuilders/DeleteQueryBuilder.php 17 19 89.47%
src/Tempest/Database/src/Builder/QueryBuilders/SelectQueryBuilder.php 82 84 97.62%
src/Tempest/Database/src/Exceptions/CannotInsertHasManyRelation.php 0 2 0.0%
src/Tempest/Database/src/Exceptions/CannotUpdateHasManyRelation.php 0 2 0.0%
src/Tempest/Database/src/Builder/ModelInspector.php 27 30 90.0%
Files with Coverage Reduction New Missed Lines %
src/Tempest/Reflection/src/TypeReflector.php 1 91.07%
Totals Coverage Status
Change from base Build 14096555604: 0.4%
Covered Lines: 10767
Relevant Lines: 13511

💛 - Coveralls

@brendt brendt changed the title refactor(database): remove DatabaseModel refactor(database): remove DatabaseModel interface Mar 26, 2025
@brendt brendt marked this pull request as ready for review March 26, 2025 13:44
@brendt
Copy link
Member Author

brendt commented Mar 26, 2025

The two failing tests are because of subcomponent changes

@erikaraujo
Copy link
Contributor

erikaraujo commented Mar 27, 2025

Introduce model definition helper, something like model($objectOrClass)->hasPrimaryKey(), model($objectOrClass)->primaryKeyName() etc.

Not sure I get this (maybe because I lack the context) - so whatever I say next can be completely wrong.

But why don't you, instead, add a #[PrimaryKey()] class attribute (for cases where the table has multi-column primary key) or and a #[Key] property attribute?

Like:

final class Book
{
    #[Key]
    public string $uuid;

    public string $title;

    //...
}

or

#[PrimaryKey(['license_plate', 'state'])]
final class Car
{
    public string $licensePlate;

    public string $state;

    public string $make;

    //...
}

of course, this would also work:

#[PrimaryKey('uuid')]
final class Book
{
    public string $uuid;

    public string $title;

    //...
}

Taking inspiration from dotnet's EF Core: https://learn.microsoft.com/en-us/ef/core/modeling/keys?tabs=data-annotations

You could also add other attributes, like Column('myWeird_columnName'), Required (or, alternatively, Nullable), etc...

@brendt brendt mentioned this pull request Mar 28, 2025
@brendt
Copy link
Member Author

brendt commented Mar 28, 2025

Yes, that's what we need to do. However, we first need to do some refactoring to make that work, because id is hardcoded in multiple places atm. It's outside the scope of this PR though: #1086

@brendt brendt merged commit 04b5bd6 into main Mar 28, 2025
65 of 67 checks passed
@brendt brendt deleted the refactor-querybuilder branch March 28, 2025 07:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ORM redesign

5 participants