Support for Easypay API 2.0 #3
Conversation
WalkthroughThe pull request introduces several modifications across multiple files to enhance the Easypay API, particularly focusing on the implementation of version 2.0. Key changes include updates to the Changes
Assessment against linked issues
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (3)
src/easypay/payment.py (2)
31-31: Remove unused importappierThe
appiermodule is imported but not used in this file, as identified by Ruff (F401). Removing unused imports improves code cleanliness.Suggested Fix:
-import appier🧰 Tools
🪛 Ruff (0.8.0)
31-31:
appierimported but unusedRemove unused import:
appier(F401)
34-42: EnsurePaymentAPImethods have necessary context and documentationWhile the
list_paymentsandcreate_paymentmethods are correctly implemented, there are a few considerations:
Dependency on
self.base_url:The methods use
self.base_url, which is not defined withinPaymentAPI. It is expected to be provided by the subclass. Ensure that any class inheriting fromPaymentAPIdefinesself.base_url.Dependency on HTTP methods
getandpost:The methods call
self.getandself.post, which should be provided by the superclass (e.g.,appier.API). Ensure thatPaymentAPIis mixed into a class that provides these methods.Lack of method docstrings:
Adding docstrings to these methods would improve code readability and maintainability by explaining their purpose and usage.
Suggested Actions:
- Add docstrings to the methods to describe their functionality.
- Document the requirements that
self.base_url,self.get, andself.postmust be provided by the subclass or superclass.Example:
class PaymentAPI(object): """ Provides payment-related API methods. Classes inheriting from PaymentAPI must define `self.base_url` and provide implementations for `self.get` and `self.post` methods. """ def list_payments(self, *args, **kwargs): """ Lists payments using the Easypay API. Args: *args: Variable length argument list. **kwargs: Arbitrary keyword arguments. Returns: A list of payments retrieved from the API. """ url = self.base_url + "single" return self.get(url, *args, **kwargs) def create_payment(self, amount, method="mb"): """ Creates a new payment. Args: amount (float): The amount for the payment. method (str, optional): The payment method. Defaults to "mb". Returns: The created payment object from the API. """ url = self.base_url + "single" return self.post(url, data_j=dict(value=amount, method=method))README.md (1)
5-9: Enhance documentation for API V2 migrationWhile the added resources are helpful, consider adding:
- Migration guide from API V1 to V2
- Authentication changes documentation
- Code examples for V2 endpoints
Example additions:
## Resources - [Easypay API Docs](https://docs.easypay.pt/) - [Easypay Admin](https://id.easypay.pt/) - [Easypay Admin Test](https://id.test.easypay.pt/) + +## Migration Guide + +### Authentication Changes +API V2 uses client_id/key authentication instead of username/password: +```python +# V1 Authentication +api = get_api() # uses username, password, cin, entity + +# V2 Authentication +api = get_api_v2() # uses client_id, key +``` + +### Example Usage +```python +# Create payment +payment = api.create_payment(amount=10.0, method="mb") + +# List payments +payments = api.list_payments() +```🧰 Tools
🪛 LanguageTool
[grammar] ~8-~8: This phrase is duplicated. You should probably use “Easypay Admin” only once.
Context: ... API Docs](https://docs.easypay.pt/) - Easypay Admin - Easypay Admin Test ## ...(PHRASE_REPETITION)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
.gitignore(1 hunks)README.md(1 hunks)src/easypay/__init__.py(1 hunks)src/easypay/base.py(3 hunks)src/easypay/payment.py(1 hunks)src/examples/__init__.py(1 hunks)src/examples/app_v2.py(1 hunks)src/examples/base.py(1 hunks)
🧰 Additional context used
🪛 Ruff (0.8.0)
src/easypay/__init__.py
32-32: .base.BASE_URL imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
32-32: .base.Scheduler imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
32-32: .base.API imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
32-32: .base.ShelveAPI imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
32-32: .base.APIV2 imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
33-33: .errors.APIError imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
33-33: .errors.SecurityError imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
34-34: .mb.MBAPI imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
35-35: .payment.PaymentAPI imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
src/examples/__init__.py
32-32: .app_v2.MBAppV2 imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
33-33: .base.get_api imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
33-33: .base.get_api_v2 imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
src/easypay/payment.py
31-31: appier imported but unused
Remove unused import: appier
(F401)
src/examples/app_v2.py
39-39: Star-arg unpacking after a keyword argument is strongly discouraged
(B026)
🪛 LanguageTool
README.md
[grammar] ~8-~8: This phrase is duplicated. You should probably use “Easypay Admin” only once.
Context: ... API Docs](https://docs.easypay.pt/) - Easypay Admin - Easypay Admin Test ## ...
(PHRASE_REPETITION)
🔇 Additional comments (3)
src/easypay/base.py (2)
45-45: Importing the payment module is necessary for PaymentAPI
The addition of from . import payment is appropriate since the APIV2 class inherits from payment.PaymentAPI. This ensures that PaymentAPI is available in the current module.
61-68: Defining BASE_URL_V2 and BASE_URL_TEST_V2 for Easypay API 2.0
The new constants BASE_URL_V2 and BASE_URL_TEST_V2 are correctly defined for the production and test environments of Easypay API 2.0. This is essential for properly routing API requests to the correct endpoints.
.gitignore (1)
6-7: Include environment files and virtual environments in .gitignore
Adding /.env and /.venv to the .gitignore file is appropriate. This prevents sensitive environment variables and local virtual environment directories from being tracked by version control.
src/easypay/base.py
Outdated
| class APIV2(appier.API, payment.PaymentAPI): | ||
| """ | ||
| Top level entry point for the Easypay API 2.0 services, | ||
| should provide the abstract implementations for the | ||
| services offered by Easypay using the 2.0 version of the API | ||
| """ | ||
|
|
||
| def __init__(self, *args, **kwargs): | ||
| appier.API.__init__(self, *args, **kwargs) | ||
| self.production = appier.conf("EASYPAY_PRODUCTION", False, cast=bool) | ||
| self.account_id = appier.conf("EASYPAY_ACCOUNT_ID", None) | ||
| self.key = appier.conf("EASYPAY_KEY", None) | ||
| self.production = kwargs.get("production", self.production) | ||
| self.account_id = kwargs.get("account_id", self.account_id) | ||
| self.key = kwargs.get("key", self.key) | ||
| self.base_url = BASE_URL_V2 if self.production else BASE_URL_TEST_V2 | ||
| self.counter = 0 | ||
| self.references = dict() | ||
| self.lock = threading.RLock() | ||
| self.scheduler = Scheduler(self) | ||
|
|
||
| def build( | ||
| self, | ||
| method, | ||
| url, | ||
| data=None, | ||
| data_j=None, | ||
| data_m=None, | ||
| headers=None, | ||
| params=None, | ||
| mime=None, | ||
| kwargs=None, | ||
| ): | ||
| appier.API.build(self, method, url, headers, kwargs) | ||
| if self.account_id: | ||
| headers["AccountId"] = self.account_id | ||
| if self.key: | ||
| headers["ApiKey"] = self.key |
There was a problem hiding this comment.
Handle headers correctly in the build method to prevent errors
There are potential issues in the build method of the APIV2 class:
-
Possible
TypeErrorifheadersisNone:The code attempts to assign values to
headerswithout ensuring it is a dictionary. IfheadersisNone, this will raise aTypeError. -
Incorrect parameter passing to the base class
buildmethod:The call to
appier.API.build()may not correctly match the expected parameters, potentially leading to unexpected behavior.
Suggested Fix:
- Ensure
headersis initialized as an empty dictionary if it'sNone. - Pass all necessary parameters to the base class
buildmethod.
Apply the following changes:
def build(
self,
method,
url,
data=None,
data_j=None,
data_m=None,
headers=None,
params=None,
mime=None,
kwargs=None,
):
+ if headers is None:
+ headers = {}
appier.API.build(
self,
method,
url,
data=data,
data_j=data_j,
data_m=data_m,
headers=headers,
params=params,
mime=mime,
kwargs=kwargs,
)
if self.account_id:
headers["AccountId"] = self.account_id
if self.key:
headers["ApiKey"] = self.keyThis ensures that headers is a dictionary and that all parameters are correctly passed to the base class method.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class APIV2(appier.API, payment.PaymentAPI): | |
| """ | |
| Top level entry point for the Easypay API 2.0 services, | |
| should provide the abstract implementations for the | |
| services offered by Easypay using the 2.0 version of the API | |
| """ | |
| def __init__(self, *args, **kwargs): | |
| appier.API.__init__(self, *args, **kwargs) | |
| self.production = appier.conf("EASYPAY_PRODUCTION", False, cast=bool) | |
| self.account_id = appier.conf("EASYPAY_ACCOUNT_ID", None) | |
| self.key = appier.conf("EASYPAY_KEY", None) | |
| self.production = kwargs.get("production", self.production) | |
| self.account_id = kwargs.get("account_id", self.account_id) | |
| self.key = kwargs.get("key", self.key) | |
| self.base_url = BASE_URL_V2 if self.production else BASE_URL_TEST_V2 | |
| self.counter = 0 | |
| self.references = dict() | |
| self.lock = threading.RLock() | |
| self.scheduler = Scheduler(self) | |
| def build( | |
| self, | |
| method, | |
| url, | |
| data=None, | |
| data_j=None, | |
| data_m=None, | |
| headers=None, | |
| params=None, | |
| mime=None, | |
| kwargs=None, | |
| ): | |
| appier.API.build(self, method, url, headers, kwargs) | |
| if self.account_id: | |
| headers["AccountId"] = self.account_id | |
| if self.key: | |
| headers["ApiKey"] = self.key | |
| class APIV2(appier.API, payment.PaymentAPI): | |
| """ | |
| Top level entry point for the Easypay API 2.0 services, | |
| should provide the abstract implementations for the | |
| services offered by Easypay using the 2.0 version of the API | |
| """ | |
| def __init__(self, *args, **kwargs): | |
| appier.API.__init__(self, *args, **kwargs) | |
| self.production = appier.conf("EASYPAY_PRODUCTION", False, cast=bool) | |
| self.account_id = appier.conf("EASYPAY_ACCOUNT_ID", None) | |
| self.key = appier.conf("EASYPAY_KEY", None) | |
| self.production = kwargs.get("production", self.production) | |
| self.account_id = kwargs.get("account_id", self.account_id) | |
| self.key = kwargs.get("key", self.key) | |
| self.base_url = BASE_URL_V2 if self.production else BASE_URL_TEST_V2 | |
| self.counter = 0 | |
| self.references = dict() | |
| self.lock = threading.RLock() | |
| self.scheduler = Scheduler(self) | |
| def build( | |
| self, | |
| method, | |
| url, | |
| data=None, | |
| data_j=None, | |
| data_m=None, | |
| headers=None, | |
| params=None, | |
| mime=None, | |
| kwargs=None, | |
| ): | |
| if headers is None: | |
| headers = {} | |
| appier.API.build( | |
| self, | |
| method, | |
| url, | |
| data=data, | |
| data_j=data_j, | |
| data_m=data_m, | |
| headers=headers, | |
| params=params, | |
| mime=mime, | |
| kwargs=kwargs, | |
| ) | |
| if self.account_id: | |
| headers["AccountId"] = self.account_id | |
| if self.key: | |
| headers["ApiKey"] = self.key |
| from .app_v2 import MBAppV2 | ||
| from .base import get_api, get_api_v2 |
There was a problem hiding this comment.
💡 Codebase verification
Add these imports to __all__ to define public interface
The imports MBAppV2, get_api, and get_api_v2 are actively used within the codebase and should be part of the module's public interface. Since __all__ is not currently defined in __init__.py, you should add it to explicitly declare these imports as part of the public API:
__all__ = ["MBAppV2", "get_api", "get_api_v2"]This will make the imports' purpose clear and follow Python's best practices for package interfaces.
🔗 Analysis chain
Unused imports detected by static analysis tools
The imports MBAppV2, get_api, and get_api_v2 are added but not used within this module. This is flagged by Ruff (F401).
Suggested Actions:
- If these imports are intended to be part of the module's public interface, consider adding them to
__all__to make them accessible when the package is imported. - If they are not needed, remove the unused imports to clean up the code.
Verification Script:
This script searches for occurrences of MBAppV2, get_api, and get_api_v2 within the src/examples/ directory to verify if they are used elsewhere.
These imports are part of the module's public interface
The imports are actively used within the codebase:
MBAppV2is defined inapp_v2.pyand used for app instantiationget_apiis used inapp.pyfor API initializationget_api_v2is used inapp_v2.pyfor API initialization
Since these imports are used by other modules and appear to be part of the package's functionality, they should be added to __all__ to explicitly define the public interface.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the imported entities are used elsewhere in the codebase.
# Search for usages of MBAppV2
rg 'MBAppV2' src/examples/
# Search for usages of get_api and get_api_v2
rg 'get_api\b' src/examples/
rg 'get_api_v2\b' src/examples/
Length of output: 613
Script:
#!/bin/bash
# Check if __all__ is already defined in __init__.py
rg "__all__" src/examples/__init__.py
Length of output: 37
🧰 Tools
🪛 Ruff (0.8.0)
32-32: .app_v2.MBAppV2 imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
33-33: .base.get_api imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
33-33: .base.get_api_v2 imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
src/easypay/__init__.py
Outdated
| from .base import BASE_URL, Scheduler, API, ShelveAPI, APIV2 | ||
| from .errors import APIError, SecurityError | ||
| from .mb import MBAPI | ||
| from .payment import PaymentAPI |
There was a problem hiding this comment.
💡 Codebase verification
These imports are part of the public API and should be added to __all__
The imports in __init__.py are intentionally re-exported for public use, as evidenced by:
- They are used in example files (src/examples/base.py uses
ShelveAPIandAPIV2) - They are part of the core API functionality (test files use them)
- The package is published to PyPI as shown in setup.py
- These classes represent the main API interfaces for the Easypay service
Add the following to src/easypay/__init__.py:
__all__ = [
"BASE_URL",
"Scheduler",
"API",
"ShelveAPI",
"APIV2",
"APIError",
"SecurityError",
"MBAPI",
"PaymentAPI"
]🔗 Analysis chain
Unused imports may need to be added to __all__ or removed
The imported entities BASE_URL, Scheduler, API, ShelveAPI, APIV2, APIError, SecurityError, MBAPI, and PaymentAPI are not used within this module, as flagged by Ruff (F401).
Suggested Actions:
- If these imports are meant to be part of the package's public API, add them to the
__all__list. - If they are not needed, remove the unused imports to clean up the code.
Verification Script:
This script helps determine whether the imports should be included in __all__ or removed.
Let me gather more information to understand how these imports are used across the codebase.
Let me check one more thing to see if these imports are used in any test files or examples, as they might be part of the public API.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the imported entities are intended for public API exposure.
# Inspect __all__ in __init__.py
rg '__all__' src/easypay/__init__.py
# List the contents of src/easypay/__init__.py to manually verify
cat src/easypay/__init__.py
Length of output: 1257
Script:
#!/bin/bash
# Check if these imported entities are used in other files that import from easypay
rg -l "from easypay import" --type py
rg "from easypay import.*?(?:BASE_URL|Scheduler|API|ShelveAPI|APIV2|APIError|SecurityError|MBAPI|PaymentAPI)" --type py
# Check if there are any relative imports of these entities
rg "from \. import.*?(?:BASE_URL|Scheduler|API|ShelveAPI|APIV2|APIError|SecurityError|MBAPI|PaymentAPI)" --type py
# Check the content of payment.py since it's imported but not shown in the initial files
cat src/easypay/payment.py
Length of output: 1668
Script:
#!/bin/bash
# Check test files for usage of these imports
fd -e py -e ipynb . test/ examples/ 2>/dev/null | xargs rg "(?:BASE_URL|Scheduler|API|ShelveAPI|APIV2|APIError|SecurityError|MBAPI|PaymentAPI)"
# Check if there's a setup.py that might indicate these are part of the public API
cat setup.py 2>/dev/null || true
Length of output: 11385
🧰 Tools
🪛 Ruff (0.8.0)
32-32: .base.BASE_URL imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
32-32: .base.Scheduler imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
32-32: .base.API imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
32-32: .base.ShelveAPI imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
32-32: .base.APIV2 imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
33-33: .errors.APIError imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
33-33: .errors.SecurityError imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
34-34: .mb.MBAPI imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
35-35: .payment.PaymentAPI imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
src/examples/base.py
Outdated
| def get_api_v2(api_class=easypay.APIV2): | ||
| return api_class( | ||
| client_id=appier.conf("EASYPAY_CLIENT_ID"), | ||
| key=appier.conf("EASYPAY_KEY"), | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for missing configuration
The new get_api_v2 function should validate the presence of required configuration parameters to prevent runtime errors.
Consider adding validation:
def get_api_v2(api_class=easypay.APIV2):
+ client_id = appier.conf("EASYPAY_CLIENT_ID", mandatory=True)
+ key = appier.conf("EASYPAY_KEY", mandatory=True)
return api_class(
- client_id=appier.conf("EASYPAY_CLIENT_ID"),
- key=appier.conf("EASYPAY_KEY"),
+ client_id=client_id,
+ key=key,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def get_api_v2(api_class=easypay.APIV2): | |
| return api_class( | |
| client_id=appier.conf("EASYPAY_CLIENT_ID"), | |
| key=appier.conf("EASYPAY_KEY"), | |
| ) | |
| def get_api_v2(api_class=easypay.APIV2): | |
| client_id = appier.conf("EASYPAY_CLIENT_ID", mandatory=True) | |
| key = appier.conf("EASYPAY_KEY", mandatory=True) | |
| return api_class( | |
| client_id=client_id, | |
| key=key, | |
| ) |
src/examples/app_v2.py
Outdated
| @appier.route("/generate/create", "GET") | ||
| def create_payment(self): | ||
| amount = self.field("amount", 10) | ||
| method = self.field("method", "mb") | ||
| return self.api.create_payment(amount=amount, method=method) |
There was a problem hiding this comment.
Security: Use POST method for payment creation
Creating payments should use POST instead of GET to:
- Prevent accidental payment creation through browser prefetching
- Follow REST conventions for resource creation
- Avoid sensitive data in URL/logs
Apply these changes:
- @appier.route("/generate/create", "GET")
+ @appier.route("/generate/create", "POST")
def create_payment(self):
amount = self.field("amount", 10)
+ if not isinstance(amount, (int, float)) or amount <= 0:
+ raise appier.ValidationError("Invalid amount")
method = self.field("method", "mb")
+ if method not in ["mb", "cc", "dd"]: # add valid payment methods
+ raise appier.ValidationError("Invalid payment method")
return self.api.create_payment(amount=amount, method=method)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @appier.route("/generate/create", "GET") | |
| def create_payment(self): | |
| amount = self.field("amount", 10) | |
| method = self.field("method", "mb") | |
| return self.api.create_payment(amount=amount, method=method) | |
| @appier.route("/generate/create", "POST") | |
| def create_payment(self): | |
| amount = self.field("amount", 10) | |
| if not isinstance(amount, (int, float)) or amount <= 0: | |
| raise appier.ValidationError("Invalid amount") | |
| method = self.field("method", "mb") | |
| if method not in ["mb", "cc", "dd"]: # add valid payment methods | |
| raise appier.ValidationError("Invalid payment method") | |
| return self.api.create_payment(amount=amount, method=method) |
| def __init__(self, *args, **kwargs): | ||
| appier.APIApp.__init__(self, name="mb_v2", *args, **kwargs) | ||
| self.api = base.get_api_v2() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix constructor parameter ordering
The constructor has star-args unpacking after keyword arguments, which is discouraged.
Apply this fix:
- def __init__(self, *args, **kwargs):
- appier.APIApp.__init__(self, name="mb_v2", *args, **kwargs)
+ def __init__(self, **kwargs):
+ appier.APIApp.__init__(self, name="mb_v2", **kwargs)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def __init__(self, *args, **kwargs): | |
| appier.APIApp.__init__(self, name="mb_v2", *args, **kwargs) | |
| self.api = base.get_api_v2() | |
| def __init__(self, **kwargs): | |
| appier.APIApp.__init__(self, name="mb_v2", **kwargs) | |
| self.api = base.get_api_v2() |
🧰 Tools
🪛 Ruff (0.8.0)
39-39: Star-arg unpacking after a keyword argument is strongly discouraged
(B026)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
src/examples/app_v2.py (1)
36-55: Consider API versioning best practices and documentationAs part of the migration to API v2.0, consider the following architectural improvements:
- Add version prefix to routes for better API versioning:
- @appier.route("/payments", "GET") + @appier.route("/v2/payments", "GET")
Add OpenAPI/Swagger documentation using appier's documentation decorators to make the API self-documenting.
Consider implementing API versioning through content negotiation using Accept headers as an alternative to URL versioning.
Would you like me to provide examples of implementing these suggestions?
🧰 Tools
🪛 Ruff (0.8.0)
39-39: Star-arg unpacking after a keyword argument is strongly discouraged
(B026)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
src/easypay/payment.py(1 hunks)src/examples/app_v2.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/easypay/payment.py
🧰 Additional context used
🪛 Ruff (0.8.0)
src/examples/app_v2.py
39-39: Star-arg unpacking after a keyword argument is strongly discouraged
(B026)
🔇 Additional comments (2)
src/examples/app_v2.py (2)
38-40: Fix constructor parameter ordering
The constructor has star-args unpacking after keyword arguments, which is discouraged.
🧰 Tools
🪛 Ruff (0.8.0)
39-39: Star-arg unpacking after a keyword argument is strongly discouraged
(B026)
46-51:
Security: Use POST method for payment creation and add input validation
Creating payments should use POST instead of GET for security reasons. Additionally, input validation is required.
- @appier.route("/payments/create", "GET")
+ @appier.route("/payments/create", "POST")
def create_payment(self):
amount = self.field("amount", 10)
+ if not isinstance(amount, (int, float)) or amount <= 0:
+ raise appier.ValidationError("Invalid amount")
method = self.field("method", "mb")
+ if method not in ["mb", "cc", "dd"]:
+ raise appier.ValidationError("Invalid payment method")
return self.api.create_payment(amount=amount, method=method)Likely invalid or redundant comment.
| @appier.route("/payments/notify", "GET") | ||
| def notify_payment(self): | ||
| print("Payment notified") | ||
|
|
There was a problem hiding this comment.
Enhance payment notification endpoint security
The notification endpoint needs proper authentication and validation to prevent unauthorized access and ensure data integrity.
- @appier.route("/payments/notify", "GET")
+ @appier.route("/payments/notify", "POST")
+ @appier.ensure_auth
def notify_payment(self):
- print("Payment notified")
+ notification_data = self.request.get_json()
+ if not self._verify_notification_signature(notification_data):
+ raise appier.SecurityError("Invalid notification signature")
+
+ payment_id = notification_data.get("payment_id")
+ status = notification_data.get("status")
+
+ # Log the notification for audit purposes
+ self.logger.info(f"Payment {payment_id} status updated to {status}")
+
+ return dict(success=True)
+
+ def _verify_notification_signature(self, data):
+ # Implement signature verification logic here
+ # using Easypay's webhook signature verification
+ return True # TODO: Replace with actual implementationWould you like me to provide the implementation for the signature verification method?
Committable suggestion skipped: line range outside the PR's diff.
| @appier.route("/payments", "GET") | ||
| def payments(self): | ||
| return self.api.list_payments() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add pagination support for payments listing
For production use, the payments endpoint should implement pagination to handle large datasets efficiently and prevent performance issues.
@appier.route("/payments", "GET")
def payments(self):
- return self.api.list_payments()
+ page = self.field("page", 1, cast=int)
+ size = self.field("size", 20, cast=int)
+ return self.api.list_payments(page=page, size=size)Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/easypay/base.py (1)
463-463: Consider v2.0-specific scheduler implementationThe current implementation reuses the v1.0 Scheduler, which is designed for MB (Multibanco) detail polling. Since API v2.0 might have different polling requirements or mechanisms, consider:
- Creating a v2.0-specific scheduler
- Documenting why v1.0 scheduler is reused if it's intentional
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/easypay/base.py(3 hunks)
🔇 Additional comments (3)
src/easypay/base.py (3)
45-45: LGTM! Well-structured constants and imports
The new imports and API endpoint constants are well-documented and follow the existing pattern of separating production and test environments.
Also applies to: 61-68
444-464: LGTM! Well-structured API v2.0 implementation
The APIV2 class is well-designed with:
- Clear separation from v1.0 implementation
- Proper configuration management
- Thread-safe implementation
465-481:
Initialize headers dictionary to prevent potential TypeError
The build method attempts to modify headers without ensuring it's initialized as a dictionary. This could raise a TypeError if headers is None.
Apply this fix:
def build(
self,
method,
url,
data=None,
data_j=None,
data_m=None,
headers=None,
params=None,
mime=None,
kwargs=None,
):
+ if headers is None:
+ headers = {}
appier.API.build(
self,
method,
url,
headers,
kwargs
)
if self.account_id:
headers["AccountId"] = self.account_id
if self.key:
headers["ApiKey"] = self.key
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/easypay/base.py (1)
61-68: Consider externalizing base URLs to configurationDefining
BASE_URL_V2andBASE_URL_TEST_V2within the code makes it less flexible to manage different environments or future URL changes. Consider externalizing these URLs to a configuration file or environment variables to enhance maintainability and adaptability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
src/easypay/__init__.py(1 hunks)src/easypay/base.py(3 hunks)src/examples/base.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/examples/base.py
🧰 Additional context used
🪛 Ruff (0.8.0)
src/easypay/__init__.py
32-32: .base.BASE_URL imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
32-32: .base.Scheduler imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
32-32: .base.API imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
32-32: .base.ShelveAPI imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
32-32: .base.APIv2 imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
33-33: .errors.APIError imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
33-33: .errors.SecurityError imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
34-34: .mb.MBAPI imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
35-35: .payment.PaymentAPI imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
🔇 Additional comments (2)
src/easypay/base.py (1)
465-481:
Handle headers correctly in the build method to prevent errors
There are potential issues in the build method of the APIv2 class:
-
Possible
TypeErrorifheadersisNone:Assigning values to
headerswithout ensuring it is a dictionary can raise aTypeErrorifheadersisNone. -
Incorrect parameter passing to the base class
buildmethod:The call to
appier.API.build()may not correctly match the expected parameters, potentially leading to unexpected behavior.
Suggested Fix:
- Initialize
headersas an empty dictionary if it'sNone. - Pass all necessary parameters to the base class
buildmethod.
Apply the following changes:
def build(
self,
method,
url,
data=None,
data_j=None,
data_m=None,
headers=None,
params=None,
mime=None,
kwargs=None,
):
+ if headers is None:
+ headers = {}
appier.API.build(
self,
method,
url,
data=data,
data_j=data_j,
data_m=data_m,
headers=headers,
params=params,
mime=mime,
kwargs=kwargs,
)
if self.account_id:
headers["AccountId"] = self.account_id
if self.key:
headers["ApiKey"] = self.keysrc/easypay/__init__.py (1)
32-35: Add imported entities to __all__ or remove unused imports
The imported entities (BASE_URL, Scheduler, API, ShelveAPI, APIv2, APIError, SecurityError, MBAPI, PaymentAPI) are not used within this module. If they are intended to be part of the public API, consider adding them to the __all__ list to explicitly export them. Otherwise, remove the unused imports to clean up the code.
🧰 Tools
🪛 Ruff (0.8.0)
32-32: .base.BASE_URL imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
32-32: .base.Scheduler imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
32-32: .base.API imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
32-32: .base.ShelveAPI imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
32-32: .base.APIv2 imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
33-33: .errors.APIError imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
33-33: .errors.SecurityError imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
34-34: .mb.MBAPI imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
35-35: .payment.PaymentAPI imported but unused; consider removing, adding to __all__, or using a redundant alias
(F401)
Fixes #1
Summary by CodeRabbit
Release Notes
New Features
PaymentAPIclass for managing payments, including methods to list and create payments.MBAppV2class for web application interface to handle payment requests.Documentation
README.mdto include a new "Resources" section with links to Easypay API documentation and admin portals.Chores
.gitignoreto exclude environment-related files for better project management.