Open-SGF / gooddads

4 stars 13 forks source link

Update model files #73

Closed jugglingdev closed 3 months ago

jugglingdev commented 3 months ago

Please review these model files.

I added @property annotations, fields, and HasUuids, HasMany, and BelongTo as appropriate.

jason-klein commented 3 months ago

Looks good overall. Nice job defining keyType and incrementing for the UUID primary keys!

1. Remove column name parameters from your belongsTo and hasMany relationships. Those are only necessary when your project does not follow standard naming conventions. For example, when you define that Child belongs to Dad, Laravel automatically assumes Child will have a dad_id field that refers to the id table on Dad.

Tip: If you ever find that you have to specify the field names to make the relationship work correctly, you are probably using the wrong relationship type. We made this mistake early in our own project.

2. Create separate use lines for each trait to make future trait additions/removals easier to review. It is much easier to see that a line has been inserted or deleted than to see what was changed within a long list of traits. For example, instead of use HasFactory, HasUuids; create two separate lines:

use HasFactory;
use HasUuids;

3. Define casts() for additional fields:

Learn more: Here's a nice SO conversation about when to add casts to fields...

By default, attributes will be casted to the type of column defined in the table. So, if your column is an integer, then it will be casted as int. But, what happens if you want to modify this behavior for specific fields? That's when attribute casting enters the scene.

4. Run composer lint to apply Laravel code standard fixes. When I forget this step before I have committed my code, I usually commit with message such as chore: composer lint or chore: lint.

jugglingdev commented 3 months ago

@jason-klein I made the updates as you explained.

I found two fields for enums: ethnicity and marital status. In the future, we will need to update these to match federal regulations.

After building out these models, I do believe it would be worthwhile to revisit the ERD as a group and re-examine defined relationships to make sure they align with business needs.

Please let me know if there are any further changes that need to be made.

Thank you!