Migrate backend of Project model to Eloquent
#1556
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
williamjallen
force-pushed
the
eloquent-project-model
branch
from
af08ed3 to
ea1b956
Compare
Collaborator
Author
|
I have marked this PR as a draft because it should not be merged until after the CDash 3.2 release is created. |
williamjallen
force-pushed
the
eloquent-project-model
branch
from
ea1b956 to
6c61a9a
Compare
williamjallen
force-pushed
the
eloquent-project-model
branch
from
6c61a9a to
74fe199
Compare
Collaborator
Author
|
The CDash 3.2 release branch has been created, and we can now proceed with CDash 3.3 PRs such as this one. |
williamjallen
force-pushed
the
eloquent-project-model
branch
2 times, most recently
from
070487a to
17884eb
Compare
josephsnyder
requested changes
Aug 17, 2023
Member
josephsnyder
left a comment
josephsnyder
left a comment
There was a problem hiding this comment.
A couple of issues with the whole "start with the only project selected" capabilities, but otherwise looks good
williamjallen
force-pushed
the
eloquent-project-model
branch
from
17884eb to
963cfd4
Compare
williamjallen
force-pushed
the
eloquent-project-model
branch
from
963cfd4 to
2b4f928
Compare
josephsnyder
approved these changes
Aug 17, 2023
Member
josephsnyder
left a comment
josephsnyder
left a comment
There was a problem hiding this comment.
Much better. Some inconsistent behavior if one tries to visit the pages without any projects created at all:
but I'm not going to lose any sleep over that,
Merged
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 1, 2023
We eventually want to convert all of the models to Eloquent. The `Build` model is the largest, and most complicated, model in the CDash codebase, and it is not feasible to simply swap it out for an Eloquent-based model. In preparation for a gradual conversion to an Eloquent backend via the adapter pattern used in #1556, I have tightened the parameter and return type constraints on the legacy Build model wherever possible. By ensuring that all of the parameter and return types are known, the process of converting individual methods to Eloquent equivalents will be easier and less bug prone. While the automated test suite passes, and I didn't see any issues during some manual testing, it is quite possible that bugs associated with this PR will pop up. By opening this PR early in the release cycle, I hope to be able to catch any resulting bugs before the CDash 3.3 release comes out.
github-merge-queue bot
pushed a commit
that referenced
this pull request
Sep 8, 2023
This PR is part of our ongoing effort to switch all of our models to Laravel's Eloquent ORM library. The legacy `SubProject` model has been turned into an adapter for the Eloquent-based model, using the same approach taken in #1556.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
Projectmodel is one of the largest models in the CDash codebase, and it would be completely impractical to convert the entire thing to Eloquent at one time. Instead, I have taken an "adapter" approach similar to the approach taken in #1539, in which the public-facing interface of the old project class remains (mostly) the same, while the business logic of most methods is outsourced to the new Eloquent-based project model.In addition to switching the code to use the Eloquent-based project when possible, I have also undertaken a general refactor of the model, deleting unused code, simplifying parameters and return types, converting all database queries to Laravel's query system, and other general maintenance tasks.
I have also completely eliminated the
projectrobottable and associated code since it is currently impossible to add new data to theprojectrobottable, and the existing data is not used for anything.Given how large of a refactor this is, this PR should not be merged until after the 3.2 release branch is created.