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 soap logs view #119

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

nicoaravena
Copy link
Collaborator

@nicoaravena nicoaravena commented Sep 20, 2022

Goal

List some SOAP commands executed from de CMS in favor to have more information in case of some purchase fails or something wrong happens.

Features

  • New view Soap Logs

Proposed changes

  • Add a new option to the executeCommand function to allow log it and add an order_id if applies.

Code

AC-CMS ADMIN

  • Base service file/handler
  • New table wp_acore_soap_logs
  • New view Soap Logs available just for WP admins
  • Filter by username and order id
  • Allow pagination

Comment on lines 141 to 143
tooltipTriggerList.forEach(function (tooltipTriggerEl) {
return new bootstrap.Tooltip(tooltipTriggerEl)
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for-of should be better of forEach in terms of performance

Suggested change
tooltipTriggerList.forEach(function (tooltipTriggerEl) {
return new bootstrap.Tooltip(tooltipTriggerEl)
});
for (const tooltipTriggerEl of tooltipTriggerList) {
new bootstrap.Tooltip(tooltipTriggerEl);
};

@Helias
Copy link
Member

Helias commented Sep 20, 2022

this feature is very good, thanks a lot! 🚀

@nicoaravena
Copy link
Collaborator Author

I just push fixes and changes.

@Helias
Copy link
Member

Helias commented Sep 22, 2022

except that raf code that has been removed, the rest seems ok to me
hope that code was dead code and it does not affect RAF, if you are sure about it we can merge this PR

thanks for this logging feature, it will be useful 🚀

@nicoaravena
Copy link
Collaborator Author

If you can check if buy an item from the store works fine, then I can merge it, but I want to have a double check.

@Helias
Copy link
Member

Helias commented Sep 22, 2022

tested char rename service and it worked but sending an item does not work, I receive an mail in the mailbox but empty, with no items :/

@nicoaravena
Copy link
Collaborator Author

@Helias did the log show anything related?

@Helias
Copy link
Member

Helias commented Sep 28, 2022

it says nothing
image

@Helias
Copy link
Member

Helias commented Sep 28, 2022

I receive an empty mail

image

@nicoaravena
Copy link
Collaborator Author

I fixed both problems

@Helias
Copy link
Member

Helias commented Oct 1, 2022

if you confirm that you tested it we can merge this PR.

So, hope you tested at least one service with sending item via mailbox and one service like char rename

@nicoaravena
Copy link
Collaborator Author

@Helias I'm not been able to retest the commands, do to some troubles in my pc with the core. Sorry for the late response.

@Helias
Copy link
Member

Helias commented Oct 24, 2022

Don't worry, If I will have time I'll test it

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.

2 participants