Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generic search/list controller #18052

Merged
merged 26 commits into from
Oct 23, 2024

Conversation

Pierstoval
Copy link
Collaborator

No description provided.

Copy link
Member

@cedric-anne cedric-anne left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This controller will probably be common to not only assets but most of the CommonDBTM classes. The technical names should not contains asset. Also, it could be considered as an index controller for the corresponding class.

  • Glpi\Controller\Asset\CommonAssetController -> Glpi\Controller\CommonIndexController (or maybe GenericIndexController)
  • /asset/{type}/search -> /{type}
  • getAssetClass() -> getClass()

@Pierstoval
Copy link
Collaborator Author

Pierstoval commented Oct 14, 2024

Instead of "index" I suggest using the term "Model", because that's both self-explanatory and generic, wdyt?

We would then have "Model list", and "Model form", for routes and controllers naming

@cedric-anne
Copy link
Member

Instead of "index" I suggest using the term "Model", because that's both self-explanatory and generic, wdyt?

We would then have "Model list", and "Model form", for routes and controllers naming

Model is already used from the functional point of view in GLPI. It refers to the hardware models. I think it would be confusing to use it here.

GenericList and GenericForm could be enough.

@Pierstoval Pierstoval force-pushed the generic-controllers branch 5 times, most recently from 8995174 to 5915e5d Compare October 14, 2024 09:39
src/CommonGLPI.php Outdated Show resolved Hide resolved
@Pierstoval Pierstoval force-pushed the generic-controllers branch 3 times, most recently from 0c4185e to c581873 Compare October 15, 2024 14:30
@Pierstoval
Copy link
Collaborator Author

For now, forms are not handled because they're much more complex.

Almost all standard cases are handled by this PR 👌

@Pierstoval Pierstoval marked this pull request as ready for review October 15, 2024 16:01
@Pierstoval Pierstoval changed the title First draft of generic asset controller Generic asset/model/entity controller Oct 15, 2024
src/Group.php Outdated Show resolved Hide resolved
src/Glpi/Controller/GenericFormController.php Outdated Show resolved Hide resolved
src/Glpi/Controller/GenericListController.php Outdated Show resolved Hide resolved
src/Glpi/Http/LegacyGenericListRouteListener.php Outdated Show resolved Hide resolved
@Pierstoval Pierstoval force-pushed the generic-controllers branch 2 times, most recently from 2b0fa27 to 7a829b4 Compare October 17, 2024 14:06
Copy link

sonarcloud bot commented Oct 18, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
11.9% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@cedric-anne cedric-anne requested review from cedric-anne and removed request for cconard96 October 23, 2024 11:03
@cedric-anne cedric-anne changed the title Generic asset/model/entity controller Generic search/list controller Oct 23, 2024
@cedric-anne cedric-anne added this to the 11.0.0 milestone Oct 23, 2024
@cedric-anne cedric-anne merged commit 4deafb1 into glpi-project:main Oct 23, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants