PriceyBot/PriceyBotPanel
Archived
5
0
Fork 0
Code Issues 6 Pull Requests Actions Packages Projects 1 Releases 4 Wiki Activity

feat/ISSUE-11-add-logging #23

Merged
llama merged 14 commits from feat/ISSUE-11-add-logging into master 2023-10-16 03:26:55 -04:00
Conversation 4 Commits 14 Files Changed 32 +871 -400
llama commented 2023-10-10 05:53:18 -04:00
Owner
Copy Link

#14

https://git.lovelynet.net/PriceyBot/PriceyBotPanel/issues/14
llama added 2 commits 2023-10-10 05:53:19 -04:00
llama added 1 commit 2023-10-10 08:48:25 -04:00
llama added 1 commit 2023-10-10 09:29:33 -04:00
kdonaldson requested changes 2023-10-11 00:37:58 -04:00
database/migrations/2023_10_10_200426_create_logs_table.php Outdated
@ -0,0 +13,4 @@
{
Schema::create('logs', function (Blueprint $table) {
$table->id();
$table->foreignIdFor(User::class)->nullable();
kdonaldson commented 2023-10-11 00:35:58 -04:00
Owner

Probably doesn't matter but might be a good idea to add constrains onto these for Foreign Keys?

Or maybe use a morphable since there's a lot of targets here?

https://laravel.com/docs/10.x/eloquent-relationships#polymorphic-relationships

Or maybe use this off the shelf library?

https://spatie.be/docs/laravel-activitylog/v4/introduction

Probably doesn't matter but might be a good idea to add constrains onto these for Foreign Keys? Or maybe use a morphable since there's a lot of targets here? https://laravel.com/docs/10.x/eloquent-relationships#polymorphic-relationships Or maybe use this off the shelf library? https://spatie.be/docs/laravel-activitylog/v4/introduction
llama commented 2023-10-11 05:42:12 -04:00
Author
Owner

I do like the ldea of morphable, I was talking with about about using something like this for transactions too when we eventually add things that aren't quotes so we can track it

I do like the ldea of morphable, I was talking with about about using something like this for transactions too when we eventually add things that aren't quotes so we can track it
llama marked this conversation as resolved
database/migrations/2023_10_10_200426_create_logs_table.php Outdated
@ -0,0 +16,4 @@
$table->foreignIdFor(User::class)->nullable();
$table->foreignIdFor(Quote::class)->nullable();
$table->foreignIdFor(RequestedQuote::class)->nullable();
$table->string('content');
kdonaldson commented 2023-10-11 00:36:21 -04:00
Owner

Unsure how long the content might need to be, but might be good as a text column?

Unsure how long the content might need to be, but might be good as a text column?
👍 1
llama marked this conversation as resolved
llama added 1 commit 2023-10-11 03:07:26 -04:00
llama added 1 commit 2023-10-11 07:28:38 -04:00
llama added 1 commit 2023-10-11 07:29:13 -04:00
kdonaldson commented 2023-10-11 18:01:44 -04:00
Owner
Copy Link

@llama
If you are going to use a morphable for this, I would highly recommend setting up a Morph Map instead of using the class names for the morphable types.

https://laravel.com/docs/10.x/eloquent-relationships#custom-polymorphic-types

That way it's not tied to the exact location of the models.

I also think you should create two nullable morphs instead of just one here, one for the "subject" and one for the "actor" / "causer", that way you can then basically log when any model causes another model to do something.

In most causes the "causer" would either be the user submitting a request or accepting the request, which right now is a single model, but could be anything going forward.

Having "Causer" be nullable should allow you to log "system" type actions as well, from crons or anything else too.

@llama If you are going to use a morphable for this, I would highly recommend setting up a Morph Map instead of using the class names for the morphable types. https://laravel.com/docs/10.x/eloquent-relationships#custom-polymorphic-types That way it's not tied to the exact location of the models. I also think you should create two nullable morphs instead of just one here, one for the "subject" and one for the "actor" / "causer", that way you can then basically log when any model causes another model to do something. In most causes the "causer" would either be the user submitting a request or accepting the request, which right now is a single model, but could be anything going forward. Having "Causer" be nullable should allow you to log "system" type actions as well, from crons or anything else too.
benjamyn commented 2023-10-12 01:27:46 -04:00
Owner
Copy Link

Checking if emojis still kill the DB 💯

Checking if emojis still kill the DB 💯
llama added 1 commit 2023-10-12 07:51:49 -04:00
llama added 1 commit 2023-10-12 07:56:00 -04:00
llama commented 2023-10-12 07:56:23 -04:00
Author
Owner
Copy Link

image
image

At some point we'll probs want to look into sorting and filtering but for now this gives up all the info we need I think

![image](/attachments/02077a5c-49f8-4073-868e-04a0c0d1c7e2) ![image](/attachments/9a9454b7-4362-4d5e-802e-bf5673504c91) At some point we'll probs want to look into sorting and filtering but for now this gives up all the info we need I think
image.png
141 KiB
image.png
135 KiB
llama added 2 commits 2023-10-12 08:10:22 -04:00
llama added 2 commits 2023-10-16 03:02:44 -04:00 
# Conflicts:
#	.github/workflows/deploy.yml
#	composer.lock
llama added 1 commit 2023-10-16 03:22:49 -04:00
llama merged commit 22338754f9 into master 2023-10-16 03:26:55 -04:00
This repo is archived. You cannot comment on pull requests.
Reviewers
No Reviewers
kdonaldson
Labels
No items
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
agoodlet balgie benjamyn kdonaldson llama
No Assignees
3 Participants
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: PriceyBot/PriceyBotPanel#23
No description provided.
Delete Branch "feat/ISSUE-11-add-logging"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?