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

Contribs fork #9

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

Conversation

rohitjoshi
Copy link
Contributor

@emreeren
I have added following features into my contrib-fork branch was was clone of your contrib branch.
What is the best way to merge with master branch?

  1. Added support for printing last ticket item using a CachePrinter
  2. Added support for creating payroll report
  3. Fixed bug in ClockIn logic. It was broken in previous merge.
  4. Fixed bug in printer helper to honor tag
  5. Added support for partial_redemption for gift card
  6. Fixed bug: Only one printing job is executed event though multiple printers are defined in printer job
  7. Added support for quick button TOGO to add a note "TOGO" to takeout order
  8. Added support to set wages for employee in User table(Partial, getting error on not null)

…1000 to accommodate merging credit card data into receipt printer.

2. Added support for new action type "Print Report" which will allow to print report based on rule. E.g I wanted to print work period report when work period ends.

3. Added support to define custom POS name in reports. I have added new program setting to set the custom pos string which is used in reports.

4. Added support for setting to choose either separate receipt for credit card or merge with ticket receipt.  Optional Signature line is printed only if ticket amount is more than specified value in setting. In US, we don't ask customer to sign receipt if amount is less than 20$.

5. Added support to add auth code, amount, name and remaning balance if gift card into credit card receipt.

6. Added support to print Terminal Name in the printer template.

7. Added support to check cash drawer status and if it is open for more than configured time, it will force user to close cash drawer.  I wanted to implement hardware independent where certain logic can be externalized in powershell script but wanted to discuss with you.  For now, it is IBM specific logic.
2. Added support for creating payroll report
3. Fixed bug in ClockIn logic
4. Fixed bug in printer helper to honor <T> tag
5. Added support for partial_redemption for gift card
6. Fixed bug: Only one printing job is executed event though multiple printers are defined in printer job
7. Added support for quick button TOGO to add a note "TOGO" to takeout order
8. Added support to set wages for employee in User table(Partial, getting error on not null)
9.
@emreeren
Copy link
Owner

Hello @rohitjoshi. Thank you very much for the commit. Let me write issues I've noticed.

  1. We can't parameterize SambaPOS brand. I request to protect and maintain our brand for the future of our project.
  2. Couldn't get the idea behind wages feature since it is commented on UserViewModel.
  3. GetPrinterMapItem shouldn't return all matched printer maps. It scans a matching map from smaller range to broader range and returns first match so we can configure it as "Drinks Category to Bar Printer and everything else to kitchen printer". For multiple printing we can configure multiple print jobs.
  4. There is no benefit with TOGO feature since it can easily implemented as a Ticket Tag button and it overwrites previously entered ticket note.
  5. Instead of modifying old migration classes we have to modify latest migration class. For example only migrations 20 and 21 works for a user who upgrades from db. version 19 to 21. Migrations less then 20 won't work for him.

Let me know your thoughts about them.

Ideally 10 new features can be 10 different pull requests. If we break it up into tiny pieces we can solve issues, merge & sync easily. And.. Your pull request is not rebased . rebase is a powerful git command for synchronizing branches. You can read about it here http://git-scm.com/book/en/Git-Branching-Rebasing

@rohitjoshi
Copy link
Contributor Author

@emreeren

  1. We can't parameterize SambaPOS brand. I request to protect and maintain our brand for the future of our project.
    I absolutely agree with you. The problem for me was when reports are printed, it shows SambaPOS and not my company name. this would confuse my CPA when sent to him for payroll. So I still prefer to specify company name on Printed reports if possible.
  2. Couldn't get the idea behind wages feature since it is commented on UserViewModel.
    Initially I was planning to calculate total pay for employee so added wages but that's not required.
    Payroll company needs to know only hours as they already maintain other employee info.
    It is not required.
  3. GetPrinterMapItem shouldn't return all matched printer maps. It scans a matching map from smaller range to broader range and returns first match so we can configure it as "Drinks Category to Bar Printer and everything else to kitchen printer". For multiple printing we can configure multiple print jobs.

I agree with you but there a bug in which printer job doesn't sent to multiple printer.
Eg. I have a printer job 'Print Bills' which is configured to send ticket to two different printers with same printer template. There is no other condition selected. Default is all * in first three columns.
Eg. One printer is CachePrinter to cache the data for later use and Other is Ticket Printer which should print immediately.
Now GetPrinterMapItem satisfies conditions for both these printers but it always returns first/default.
So it only prints on the first printed defined in the print job.
So as a workaround, I added additional GetPrinterMapsItem which returns both the printer which satisfied condition.
But as you described, it will break other filtering.
What is the best way to fix this?

  1. There is no benefit with TOGO feature since it can easily implemented as a Ticket Tag button and it overwrites previously entered ticket note.
    I agree. It is a duplicate button and everyone doesn't need it.
    Just to optimize cashier's time in writing Togo with soft keyboard, I have added this button.
    This is existing buttons in my legacy cash register and thought of supporting it.
  2. Instead of modifying old migration classes we have to modify latest migration class. For example only migrations 20 and 21 works for a user who upgrades from db. version 19 to 21. Migrations less then 20 won't work for him.
    Agree. My mistake.

Let me know if I should make changes as you recommended before you merge. I am done with my code changes for new features. I am planning to test my POS next two weeks before start using it.

Thanks a lot for your help and I am happy I was able to contribute for the community.

2. Synchronized timer object for cashdrawer open notification
@emreeren
Copy link
Owner

emreeren commented Apr 1, 2013

Hello @rohitjoshi. Sorry for responding late. You can make modifications and commit your changes on this fork. I'm on a trip and I'll merge them when I'm back.

For multiple printing cases we execute "Execute Print Job" action multiple times. Generally we use a dummy ticket tag button for executing it. Creating "Print Job Executed" rule might be a better solution so we can understand a print job executed by hand or automatically and we can execute related print jobs. Of course if you think a better solution we can talk about it. I'll also continue searching a better solution but we shouldn't modify map matching code for now.

We can easily solve branding issue by leaving screen header label as "Samba POS". On report part we can read company info as you did.

I'll fix migration issues just before releasing public setup. You can add database modifications as a reminder comment on latest migration class.

Let me know when you need assistance. I'll try to respond faster. I hope to come back at weekend.

@rohitjoshi
Copy link
Contributor Author

Hello @emreeren,
I will continue fixing some of the issues before we merge.

  1. For multiple printing, I guess "Print job Executed" would work but I thought adding multiple print jobs would be more efficient. also if you look at the code, it still executes logic to validate if given printer matches specified criteria. In addition, it checks if more than one printer matches, it will add job for that printer as well.
  2. Regarding branding, sorry, I replaced "Samba Pos" globally rather than just using it in the report. I will fix it.
  3. As Wages fields in Users table is no longer required, no migration changes required.

Will let you know once I am done with some bug fixes.

2. Fixed bug in line printer which try to align and throw exception. without this fix, it terminats application.
3. Added support for internation format of date
4. Fixed bug in when external credit card payment selected
5. Removed Wages tag from user
6. Fixed bug in reading the cash drawer status
7. On exception, close the port.
2. Set the credit card transaction type to "External" when external button selected
fix +/- qunatity button and pole display
fixed ticket tag display for pole display
Fixed display of change due when tendered amount is less than due
Added utility for IBMSurePOS Cash Register
…ct.cs:13 Destination array was not long enough
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