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

KNJ-17025 Add the support for customer object for payment/oneclick in… #2

Merged
merged 1 commit into from
Jul 22, 2024

Conversation

HonzaSaibic
Copy link

…it method.

@HonzaSaibic HonzaSaibic requested review from Formulka, rubickcz and xdaniel3 and removed request for rubickcz July 15, 2024 12:28
@@ -40,7 +40,7 @@ def __init__(self, merchant_id, base_url, private_key, csob_pub_key):

:param merchant_id: Your Merchant ID (you can find it in POSMerchant)
:param base_url: Base API url development / production
:param private_key: Path to generated private key file, or its contents
:param private_key: CSOB private key string
Copy link
Author

Choose a reason for hiding this comment

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

This comment was incorrect from a previous PR. We no longer pass the file to the private_key parameter.

Comment on lines -39 to -45
def test_client_init_can_take_key_path(self):
client = CsobClient(merchant_id='MERCHANT',
base_url=BASE_URL,
private_key=KEY_PATH,
csob_pub_key=KEY_PATH)
assert client.key == self.key
assert client.pubkey == self.key
Copy link
Author

Choose a reason for hiding this comment

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

This test has been failing since the last version.

@@ -25,7 +26,7 @@ def setUp(self):
self.key = open(KEY_PATH).read()
self.c = CsobClient(merchant_id='MERCHANT',
base_url=BASE_URL,
private_key=KEY_PATH,
private_key=self.key,
Copy link
Author

Choose a reason for hiding this comment

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

As said in the previous comment, we no longer pass private key as a file.

@@ -123,6 +124,7 @@ def payment_init(self, order_no, total_amount, return_url, description, merchant
('payMethod', 'card'),
('totalAmount', total_amount),
('currency', currency),
('customer', utils.convert_keys_to_camel_case(customer_data)),
Copy link
Author

Choose a reason for hiding this comment

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

assert to_camel_case("THIS_IS_SNAKE_CASE") == "thisIsSnakeCase"
assert to_camel_case("thisIsSnakeCase") == "thisIsSnakeCase"

def test_convert_keys_to_camel_case_should_convert_dict_keys_to_camel_case(self):

Choose a reason for hiding this comment

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

chybí testcase s listem

Copy link
Author

Choose a reason for hiding this comment

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

přidáno 👍

pycsob/utils.py Outdated
@@ -121,3 +121,31 @@ def get_card_provider(long_masked_number):
if rx.match(long_masked_number[:6]):
return provider_id, conf.CARD_PROVIDERS[provider_id]
return None, None


def to_camel_case(value):

Choose a reason for hiding this comment

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

typing u obou funkcí

Copy link
Author

Choose a reason for hiding this comment

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

doplněno 🙏

pycsob/utils.py Outdated
return "".join([first_word.lower(), *map(str.title, other_words)]) if other_words else first_word


def convert_keys_to_camel_case(data):
Copy link

@xdaniel3 xdaniel3 Jul 16, 2024

Choose a reason for hiding this comment

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

Chová se to docela zvláštně, podle toho jestli tam dáš list nebo dict tak to pak vrací list nebo dict. Tohle se bude aji těžko typovat. Snažil bych se to sjednotit na 1 datovou strukturu -> list.

Copy link
Author

Choose a reason for hiding this comment

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

Mně to funguje dobře. typ je dict[str, Any]. Má to convertovat POUZE klíče z dictu, takže když tam dáš list, tak ti to má vrátit hodnoty tak jak jsou, proto i ve jméně key.

Pokud tam dám list, který obsahuje dict, tak to vrátí list s dictem 🤔

Copy link
Author

Choose a reason for hiding this comment

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

když tam dáš list stringů, tak to nemá nic udělat, to je expected 👍

Choose a reason for hiding this comment

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

def convert_keys_to_camel_case(data: list[Any]) -> list[Any]:
    if not data:
        return data
    
    result = []
    for el in data:
        if isinstance(el, dict):
            result.append(
                {to_camel_case(key): value for key, value in el.items()}
            )
        elif isinstance(el, list):
            result.append(convert_keys_to_camel_case(el))
        else:
            log.warning("Invalid value")
            result.append(el)
        
    return result

řešení pouze s listem, více řádků, ale žádný copy paste a jasný interface, není tam mapping mezi typem argumentu a návratovou hodnotou. Nicméně ber to jako nitpick :-)

Copy link
Author

Choose a reason for hiding this comment

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

Tohle řešení nefunguje úplně spolehlivě a hlavně by se s tím pak špatně a neintuitivně pracovalo pro případ, který potřebujeme. Jestli nevadí, nechal bych takto 🙏

Choose a reason for hiding this comment

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

the function should accept list and dict and return the same type it is called with, typing can be solved through generics

T = TypeVar("T", list, dict)
def convert_keys_to_camel_case(data: T) -> T:
    ...

Choose a reason for hiding this comment

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

Good idea with generics :-) Agreed

@HonzaSaibic HonzaSaibic force-pushed the KNJ-17025 branch 9 times, most recently from 304705f to 2aa7985 Compare July 16, 2024 12:42
@xdaniel3 xdaniel3 merged commit 21bb597 into skip-pay:master Jul 22, 2024
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.

3 participants