[ADVAPP-1152]: Refactor the data models for telephone numbers, email addresses, physical addresses. #1285
Conversation
Orrison
changed the title
|
@ankit-canyon this had the wrong ticket number. I have corrected it in the title and description. |
| { | ||
| Schema::create('student_email_addresses', function (Blueprint $table) { | ||
| $table->uuid('id')->primary(); | ||
| $table->string('student_id'); |
There was a problem hiding this comment.
Oops, sorry. We should keep this consistent with the rest of the app and have this column called sisid.
And it should be a foreign key constrained to the students table, cascading on delete.
| { | ||
| Schema::create('student_phone_numbers', function (Blueprint $table) { | ||
| $table->uuid('id')->primary(); | ||
| $table->string('student_id'); |
There was a problem hiding this comment.
Oops, sorry. We should keep this consistent with the rest of the app and have this column called sisid.
And it should be a foreign key constrained to the students table, cascading on delete.
| { | ||
| Schema::create('student_addresses', function (Blueprint $table) { | ||
| $table->uuid('id')->primary(); | ||
| $table->string('student_id'); |
There was a problem hiding this comment.
Oops, sorry. We should keep this consistent with the rest of the app and have this column called sisid.
And it should be a foreign key constrained to the students table, cascading on delete.
| @@ -324,6 +324,21 @@ public function basicNeedsPrograms(): MorphToMany | |||
| )->withTimestamps(); | |||
| } | |||
|
|
|||
| public function studentAddress(): HasMany | |||
| { | |||
| return $this->hasMany(StudentAddress::class, 'student_id'); | |||
There was a problem hiding this comment.
This will need to be updated when student_id is changed to sisid.
|
|
||
| public function studentEmailAddress(): HasMany | ||
| { | ||
| return $this->hasMany(StudentEmailAddress::class, 'student_id'); |
There was a problem hiding this comment.
This will need to be updated when student_id is changed to sisid.
| 'state' => $this->faker->state, | ||
| 'postal' => $this->faker->postcode, | ||
| 'country' => $this->faker->country, | ||
| ]; |
There was a problem hiding this comment.
When we add the type here the random choices can be "Home", "Dorm", and "Work"
| 'number' => $this->faker->phoneNumber, | ||
| 'ext' => $this->faker->randomNumber(), | ||
| 'type' => $this->faker->words(10), | ||
| 'is_mobile' => $this->faker->boolean, |
There was a problem hiding this comment.
Please replace each $this->faker with fake()
| return [ | ||
| 'prospect_id' => Prospect::factory(), | ||
| 'number' => $this->faker->phoneNumber, | ||
| 'ext' => $this->faker->randomNumber(), |
There was a problem hiding this comment.
Let's make this null in the definition. And then add a state for withExtension
| 'prospect_id' => Prospect::factory(), | ||
| 'number' => $this->faker->phoneNumber, | ||
| 'ext' => $this->faker->randomNumber(), | ||
| 'type' => $this->faker->words(10), |
There was a problem hiding this comment.
Rather than having this be random words. Let's make this a random element selection between: "Home", "Mobile", "Work".
| 'number' => $this->faker->phoneNumber, | ||
| 'ext' => $this->faker->randomNumber(), | ||
| 'type' => $this->faker->words(10), | ||
| 'is_mobile' => $this->faker->boolean, |
There was a problem hiding this comment.
Let's add two states to this Factory. mobile and notMobile to set this true or false, respectively. But this random generation can stay in the definition.
ankit-canyon
force-pushed
the
features/advapp-1152
branch
from
50558f3 to
4e1c914
Compare
ankit-canyon
force-pushed
the
features/advapp-1152
branch
from
4e1c914 to
eed88f1
Compare
There was a problem hiding this comment.
@ankit-canyon I am just marking this so that you can re-request review when you consider Phase 2 complete
…mprove factory definitions
…ne number models and update factories accordingly
…nd students tables
…r prospects and students
ankit-canyon
force-pushed
the
features/advapp-1152
branch
from
81ecc7d to
e9a37a3
Compare
ankit-canyon
force-pushed
the
features/advapp-1152
branch
from
1cd1adc to
903b3f7
Compare
|
Ticket(s) or GitHub Issue
Technical Description
Any deployment steps required?
Are any Feature Flags and/or Data Migrations that can eventually be removed Added?
Before contributing and submitting this PR, make sure you have Read, agree, and are compliant with the contributing guidelines.