[9.0][IMP] pos_payment_terminal: better pywebdriver interface by sbidoul · Pull Request #3 · acsone/pos

sbidoul · 2019-10-30T17:07:02Z

Better pywebdriver interface in the case the driver can obtain transaction statuses from the terminal.

sbidoul · 2019-10-30T17:08:13Z

+            if ('transaction_id' in response){
+                line.transaction_id = response['transaction_id']
+            }


You can simplify by writing line.transaction_id = response.transaction_id, no need to test for the field presence.

sbidoul · 2019-10-30T17:13:35Z

            }
-            this.update_transaction_data(line, data)
-            this.message('payment_terminal_transaction_start', {'payment_info' : JSON.stringify(data)});
+            self.update_transaction_data(line, data)


Suggested change

self.update_transaction_data(line, data)

self.update_transaction_data(line, data);

sbidoul · 2019-10-30T17:16:38Z

+            vals['transaction_id'] = this.transaction_id || false;
+            vals['success'] = this.success || false;
+            vals['status'] = this.status || false;
+            vals['reference'] = this.reference || false;


|| false sounds suspicious. Because for sucess, we want to distinguish (null/undefined, false, true).

I'd say you can simply do
vals.transaction_id = this.transaction_id

codecov-io · 2019-10-30T17:19:38Z

Codecov Report

Merging #3 into 9.0-add_pos_payment_terminal-dro will increase coverage by 0.16%.
The diff coverage is 100%.

@@                         Coverage Diff                          @@
##           9.0-add_pos_payment_terminal-dro       #3      +/-   ##
====================================================================
+ Coverage                             55.67%   55.83%   +0.16%     
====================================================================
  Files                                    20       20              
  Lines                                   273      274       +1     
====================================================================
+ Hits                                    152      153       +1     
  Misses                                  121      121

Impacted Files	Coverage Δ
pos_payment_terminal/models/pos_config.py	`100% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4af7c9f...724dcac. Read the comment docs.

Payment lines are updated with response details Amount due is updated if 'success' == True in response

…ion and status

sbidoul · 2019-11-07T10:59:14Z

+    var screens = require('point_of_sale.screens');
+
+    screens.PaymentScreenWidget.include({
+        deactivate_next: function(){


Suggested change

deactivate_next: function(){

deactivate_validate_button: function(){

sbidoul · 2019-11-07T10:59:49Z

+        deactivate_next: function(){
+            this.$('.next').toggle(false);
+        },
+        activate_next: function(){


Suggested change

activate_next: function(){

activate_validate_button: function(){

sbidoul · 2019-11-07T11:07:04Z

+            });
+            paymentwidget.order_changes();
+        },
+        update_payment_line: function(response){


Suggested change

update_payment_line: function(response){

update_payment_line: function(driver_status){

sbidoul · 2019-11-07T11:12:22Z

+                        if ('reference' in transaction){
+                            line.terminal_transaction_reference = transaction['reference'];
+                        }
+                        if (line){


this if line seems redundant

sbidoul · 2019-11-07T11:16:18Z

+            if ('transaction_id' in response){
+                line.terminal_transaction_id = response['transaction_id']
+            }
+            if ('success' in response){


this test seems useless: line.terminal_success = response.success should be sufficient.

sbidoul · 2019-11-07T11:26:36Z

+            }
+            else if (line.terminal_transaction_success === true){
+                self.activate_next();
+            }


handle all this button state inside render_payment_line

sbidoul · 2019-11-07T11:28:20Z

+        on_click_transaction_start: function(event){
+            var line_cid = $(event.currentTarget).data('cid');
+            var self = this;
+            self.hide_transaction_started(line_cid);


do this in render_payment_line to have all the rendering logic in one place

sbidoul · 2019-11-08T10:50:46Z

+                var payment_lines = self.get_paymentlines();
+                for (var id in payment_lines){
+                    var payment_line = payment_lines[id]
+                    if payment_line.in_transaction{


I don't think we need in_transaction: it should be equivalent to (transaction_id is not null and transaction_success is null).

[9.0][IMP] pos_payment_terminal: Additional data to payment lines

897c881

sbidoul commented Oct 30, 2019

View reviewed changes

acsone-git-bot added the needs review label Oct 30, 2019

rousseldenis added 2 commits October 31, 2019 12:34

[9.0][IMP] pos_payment_terminal: Update payment lines with response

a4f09df

Payment lines are updated with response details Amount due is updated if 'success' == True in response

[9.0][IMP] pos_payment_terminal: Improvements to support line protect…

f0790da

…ion and status

rousseldenis mentioned this pull request Nov 4, 2019

[9.0][IMP] pos_payment_terminal_server: take into account changes in … #4

Open

1 task

[9.0][IMP] pos_payment_terminal: Protect Validate button

145be15

rousseldenis force-pushed the 9.0-imp-pos_payment_terminal_enhanced-dro branch from 94fefb5 to 145be15 Compare November 6, 2019 13:30

rousseldenis changed the base branch from 9.0 to 9.0-add_pos_payment_terminal-dro November 7, 2019 10:53

sbidoul commented Nov 7, 2019

View reviewed changes

rousseldenis force-pushed the 9.0-imp-pos_payment_terminal_enhanced-dro branch from 91a0862 to 631d54f Compare November 8, 2019 10:43

sbidoul commented Nov 8, 2019

View reviewed changes

rousseldenis force-pushed the 9.0-imp-pos_payment_terminal_enhanced-dro branch 12 times, most recently from 6ab341e to 9d84324 Compare November 8, 2019 14:01

[WIP] changes from review

0c32d13

rousseldenis force-pushed the 9.0-imp-pos_payment_terminal_enhanced-dro branch 3 times, most recently from 8db0e32 to 0d6ccba Compare November 8, 2019 15:01

!fixup

b86459f

rousseldenis force-pushed the 9.0-imp-pos_payment_terminal_enhanced-dro branch from 0d6ccba to b86459f Compare November 8, 2019 15:05

rousseldenis added 3 commits November 13, 2019 12:33

!fixup

ab7e8d4

!fixup

67b7e4c

!fixup

724dcac

	self.update_transaction_data(line, data)
	self.update_transaction_data(line, data);

	deactivate_next: function(){
	deactivate_validate_button: function(){

	activate_next: function(){
	activate_validate_button: function(){

	update_payment_line: function(response){
	update_payment_line: function(driver_status){

Conversation

sbidoul commented Oct 30, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov-io commented Oct 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov-io commented Oct 30, 2019 •

edited

Loading