Download Student Applications as PDF (background process) and Excel #200
Conversation
…and adds docblocks
…nto bulk-export-student-apps-pdf
betsyecastro
force-pushed
the
bulk-export-student-apps-pdf
branch
from
b05ac6f to
9b3f3de
Compare
betsyecastro
force-pushed
the
bulk-export-student-apps-pdf
branch
from
8ba4fce to
dc5330d
Compare
betsyecastro
force-pushed
the
bulk-export-student-apps-pdf
branch
from
dc5330d to
a13215e
Compare
shukla-m
left a comment
shukla-m
left a comment
There was a problem hiding this comment.
Hi @betsyecastro, this works great! I tested both bulk PDF downloads (with and without filters) and individual PDF download and everything seems to work as expected. I added a few comments for minor changes. Let me know if you have any questions. Thanks.
There was a problem hiding this comment.
Is app\Jobs\CleanupPdfFiles.php being used anywhere? It looks like app\Console\Commands\CleanUpPdfFiles.php is used in Kernel.php, but I didn't see this file being used anywhere, unless I missed it.
| { | ||
| public function generate(array $payload): PdfGenerationResult | ||
| { | ||
|
|
There was a problem hiding this comment.
Is the LambdaPdfHelper implementation included here for future reference, since we are not currently using lambda?
Also, do we need the commented lines below for Pdf::view() invocation?
|
|
||
| namespace App\Http\Controllers; | ||
|
|
||
| use App\Profile; |
There was a problem hiding this comment.
It looks like the following imports are not being used and can be removed: Profile, Student, Model, Storage.
| use App\Helpers\Contracts\PdfGenerationHelperContract; | ||
| use App\Helpers\LambdaPdfHelper; | ||
| use App\Macros\CollectionMacros; | ||
| use App\Services\PdfGenerationService; |
There was a problem hiding this comment.
Looks like PdfGenerationService and Log are not being used in this file so the imports can be removed.
| } | ||
|
|
||
| $this->app->bind(PdfGenerationHelperContract::class, function () { | ||
| if (config('pdf.driver') === 'lambda') { |
There was a problem hiding this comment.
Is this code for future reference in case we use lambda at some point?
| */ | ||
|
|
||
| 'default' => env('QUEUE_DRIVER', 'sync'), | ||
| 'default' => env('QUEUE_DRIVER', 'database'), |
There was a problem hiding this comment.
If possible and if it makes sense, could you please add a comment on why it was changed from sync to database? It might be helpful for future reference. Thanks.
| use Illuminate\Foundation\Bus\Dispatchable; | ||
| use Illuminate\Queue\InteractsWithQueue; | ||
| use Illuminate\Queue\SerializesModels; | ||
| use App\Events\PdfReady; |
There was a problem hiding this comment.
It looks like the following imports are not being used and can be removed: PdfReady, Student, URL.
2 participants
Adds support in the profile-students view for downloading both individual and multiple student applications. The "Download" option allows downloading a single application, while "Download Student Applications" enables exporting filtered applications by filing status in Excel or PDF formats to address issue #187
The job that generates the PDF file in the background, receives a
PdfGeneratorService- an implementation of thePdfGenerationContractinterface to support multiple PDF generation methods Browsershot, AWS Lambda, and container-based).Replaces PR #192.