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

Allow reporting by a list of order numbers #104

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

Conversation

cebe
Copy link
Collaborator

@cebe cebe commented Jan 17, 2023

fixes #86

image

image

Open Questions:

  • Should the form "by Order #" also include the list of customers? If not I need to add a permission check to limit the list to the list of customers visible to a user.
  • Currently the isAdmin check on the user does never return true so I'm not sure how the list of customers would look like in live system.

@cebe cebe marked this pull request as draft January 17, 2023 10:02
@cebe cebe requested a review from cgsmith January 17, 2023 10:02
Copy link
Contributor

@cgsmith cgsmith left a comment

Choose a reason for hiding this comment

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

Great work so far @cebe :). I'll share the Digital Ocean info with you for testing since the report job uploads to digital ocean if I remember correctly.

@cgsmith
Copy link
Contributor

cgsmith commented Jan 17, 2023

Should the form "by Order #" also include the list of customers? If not I need to add a permission check to limit the list to the list of customers visible to a user.

No need to include list of customers, limiting the list is fine. Most customers only have a 1:1 association

Currently the isAdmin check on the user does never return true so I'm not sure how the list of customers would look like in live system.

Are you referring to Yii::$app->user->identity->getCustomerList() if the isAdmin check fails?

@cgsmith cgsmith added the enhancement New feature or request label Jan 17, 2023
@cebe
Copy link
Collaborator Author

cebe commented Feb 15, 2023

  • Currently the isAdmin check on the user does never return true so I'm not sure how the list of customers would look like in live system.

checked both cases, should be fine.

@cgsmith
Copy link
Contributor

cgsmith commented Feb 16, 2023

I tried testing the order report but get this error on the job:

> Error: Typed property console\jobs\CreateReportJob::$start_date must not be accessed before initialization

@cebe
Copy link
Collaborator Author

cebe commented Feb 21, 2023

Error: Typed property console\jobs\CreateReportJob::$start_date must not be accessed before initialization

fixed.

@cebe cebe force-pushed the 86-report-by-order-nr branch from 3a93fd6 to df57695 Compare February 21, 2023 13:43
@cebe cebe marked this pull request as ready for review February 21, 2023 13:43
@cebe cebe requested a review from cgsmith February 21, 2023 13:44
@cebe cebe changed the title WIP: Allow reporting by a list of order numbers Allow reporting by a list of order numbers Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Ability to generate a report on specific orders
2 participants