EASOL / easol

EASOL - A New Way to Open Learning with Ed-Tech
http://easol.org
GNU Affero General Public License v3.0
1 stars 4 forks source link

Start converting Custom Queries to ORM #374

Open edgarf opened 7 years ago

edgarf commented 7 years ago

We need to start getting rid of custom queries and using ORM (where possible!)

We should always be careful and make sure the output doesn't change after we replace the custom queries.

skoshkarev commented 7 years ago

@regiscamimura @edgarf

Q1: Why we have different folders "application/models/orm" and "application/models/entities"? I still don't get it.

Q2: Can you confirm that the folder "application/orm/easol" is the place where I have to put my new ORM classes?

edgarf commented 7 years ago

@regiscamimura correct me if I'm wrong, but I think application/models/entities is old implementation, which we should not maintain. You should be adding new stuff into application/models/orm

regiscamimura commented 7 years ago

@skoshkarev Edgar is right, application/models/entities is old stuff, we're abandoning it.

Said, that, be aware that using the ORM may require changes in the views. For example, if you set the relationships right on the ORM, you don't need to use joins to get related data. You can do something like:

$Student = Model\Edfi\Student::find($StudentUSI); Then, if you need to get related data, you can do in the view:

<div><?php echo $Student->StudentAddress()[0]->City; ?></div>

The database is very complex though, sometimes you need to work on too many relationships to get the data you need. For example, if you need to get which school a student is related to, you'll need to work with a many-to-many relationship, so there is a table between student and school, and a school is related to a "educational organization", so you need to refer to that relationship too, and using the ORM may not be that easy from the view, I mean, we would have something like

<div><?php echo $Student->School()[0]->EducationalOrganization()->Title; ?></div>

That's just an example, the field and model names are probably not accurate, but you got the idea.

In the other hand, creating the query result on having all those big complex queries for simple things.

What I think we should do is to create an intermediate model that can use either raw queries or ORM. Raw queries should be avoided anyway, please use ORM whenever you can, avoid raw queries. They are not forbidden though.

So, create new models directly in the application/models folder. You see we have some files there like Easol_Helper.php, Schoomanagement_m.php (that's bad), but the idea is also to ger rid of those, so just create new models in the models folder, along with those files. ONLY if needed, if we can use the classes in the orm folder, that's better.

For an example of area using only ORM, check the user management area. The controllers are in application/controllers/management/user.php, and application/views/management/user.

If you do need to create the intermediate model, please name them like models/[Tablename_model].php, following codeigniter's default naming pattern. The class there should use/call the ORM classes whenever you can, just by using their namespaces but do NOT extend the ORM classes -- I'll explain why later. This intermediate model should be the purpose of making the ORM easier to use on controller and view end, so it's more or less like a "shortcut" for acessing complex ORM data structure.

For those intermediate models, please use this naming convention:

function listing($filter=[]) -- for listing record, the $filter is an optional parameter so you can pass arguments to the query. function [related]_listing($fk) - for listing records related to the model, for example, Edfi.Staff has multiple email addresses, so you could have a Staff_model, which would have a main "listing()" function, to get a list with all the Staff records, and then a email_listing($StaffUSI), to get a list of all email addresses related to a given Staff record (which PK is StaffUSI).

function save($pk=null) -- for saving a record. This function should be able to either do a insert or an update in the database. Run an update if a PK is provided. function delete($pk) -- You know what it means, right? function get($filter=[]) -- to get a single record. $filter can be a string, int, or array. If not an array, consider the parameter is the model's PK, otherwise run a query with all the arguments in the $filter array.

Those intermediates models should extend the core model for application/core/Easol_model.php. You'll find there a model that have the methods above, and so when you create an intermediate model, you can extend it and just use those methods. That core model does not uses ORM though, so although we can use its methods, please override then with ORM approach the most you can.

Makes sense?

regiscamimura commented 7 years ago

Regarding new ORM, please put them on application/models/orm/easol, if the model is for EASOL. tables, and models/orm/edfi otherwhise.