wpovernight / woocommerce-pdf-invoices-packing-slips

Create, print & automatically email PDF invoices & packing slips for WooCommerce orders.
https://wordpress.org/plugins/woocommerce-pdf-invoices-packing-slips/
Other
100 stars 46 forks source link

Improve document numbers logging #838

Closed BrunoPavlinic98 closed 1 month ago

BrunoPavlinic98 commented 1 month ago

closes #809

alexmigf commented 1 month ago

@MohamadNateqi @BrunoPavlinic98

I'm not sure if this improvement is effective. It seems like all the errors we're seeing are common across every document, so we're ending up with different logs that show the same errors. I believe it might be more useful to have separate logs for main objects like settings, documents, and so on.

What do you think?

MohamadNateqi commented 1 month ago

@alexmigf I also agree that this improvement does not seem to be so effective.

I believe it might be more useful to have separate logs for main objects like settings, documents, and so on.

Could you please explain your idea more? How does it help in logging document numbers?

alexmigf commented 1 month ago

To log only the document numbers, I'm uncertain if this implementation is correct. However, we do have the number tools table. What impact will this have?

MohamadNateqi commented 1 month ago

Maybe we can have separate logs for each document number, and also, update the current log to collect more data and maybe registering new logs?

alexmigf commented 1 month ago

have separate logs for each document number

You will end up with a large number of log files, which doesn't seem practical.

update the current log to collect more data and maybe registering new logs

What data do you think is missing or should be recorded that isn't currently?

MohamadNateqi commented 1 month ago

You will end up with a large number of log files, which doesn't seem practical.

I think I didn't express myself well. I meant one log file for each document type, just for numbers, like ...invoice-numbers.

What data do you think is missing or should be recorded that isn't currently?

In the recent issue we had in the initiate_number(), we registered several more logs just to track the flow. Maybe we can register some of the logs we did that time for the test? However, I don't recall it right now, and I need to check the code.

But besides this, what's your idea?

alexmigf commented 1 month ago

I think we have a good amount of information right now:

If any error occurs when getting the number it should be displayed in the general log file I think.

MohamadNateqi commented 1 month ago

You're right. I also agree. So this PR and the issue need to be closed?

alexmigf commented 1 month ago

This is just my opinion, but maybe we could improve the numbers DB table instead of removing numbers from it. For example, we could add an active column. I think you also suggested something similar in the past. We can discuss this further in a new issue.

I don't think we need more logs at this stage.

MohamadNateqi commented 1 month ago

Agree. It seems to have enough logs.

This is just my opinion, but maybe we could improve the numbers DB table instead of removing numbers from it. For example, we could add an active column. I think you also suggested something similar in the past. We can discuss this further in a new issue.

Yes, I think I mentioned adding the soft delete ability instead of removing the row from the DB. This can help us track what happened exactly and could also be displayed in the Number tools. We can consider the "active" column you mentioned and the "soft delete".

alexmigf commented 1 month ago

Yes, let's open a new issue to discuss that, but it's not urgent.

MohamadNateqi commented 1 month ago

Should I open the new issue?

alexmigf commented 1 month ago

If we don't have anything related already opened yes.

BrunoPavlinic98 commented 1 month ago

@alexmigf @MohamadNateqi Agreed. I will close this issue and PR.