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

Feature - Solve Exercises 1 and 2 #9

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Armandisimo-Git
Copy link

@Armandisimo-Git Armandisimo-Git commented Feb 2, 2022

Motivation

The following PR was raised in order to resolve the exercises assigned and outlined in this document.

Exercise 1

The new attachment implementation was created by first running the command php artisan make:model Attachment -m creating both model and migration. Then i proceeded to create the relationships in the respective models (Post, Comment and Attachment).

The feature test was performed to test the new console command, ensuring the data. In the logic of the command, a progress bar was used to have a visual guide of how the migration proceeds, in addition, the data array was segmented so that there were no database errors due to the number of records that are being migrated.

console-command

Total data in attachments is 9150 records after the migration with a total of 92 inserts of 100 records each; in this way errors are avoided if a large number of records are migrated.

image

In this image you can see that there are a number of 9152 assertions of which 9150 correspond to the number of records that should be migrated, the test is heavier than it should be but I wanted to make sure that all the data was inserted correctly in the attachments table.

Steps to follow:

1. run php artisan migrate:fresh --seed

2. run php artisan migrate_attachments_data

Exercise 2

For this exercise, the feature test was created first, following the TDD approach; In this way, I proceeded to change in the index view the relationships that called the old models (post_attachments, comment_attachments) and replaced them with the new relationship with attachments that was implemented in exercise 1; and updated the IndexViewTest again to reflect this change.

index1

Before refactoring. Queries 656, Models 555 and 460ms.

index2

First change. 141ms faster only using polymorphic relation (Attachment) from the Exercise 1.

console-command2

IndexViewTest passes after these changes.

The next thing that was done to finish the exercise was to move the logic of the route to a controller (HomeController), optimize the query using Eager Loading, adding a new relationship in Post model (hasManyThrough) to access the attachments table more directly , at the same time we save database queries, load fewer models and minimize response time as much as possible on the backend side. Finally we change the way the data is printed in the index view to reflect the changes from the backend.

index4

After refactoring. Queries 2, Models 55 and 11ms.

tests

Tests suite.

@Armandisimo-Git Armandisimo-Git changed the title Solved Exercise 1 and Exercise 2 Feature - Solve Exercises 1 and 2 Feb 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant