Added Paydisini Integration #309
Conversation
Reviewer's Guide by SourceryThis pull request integrates the Paydisini payment system into the MissKaty bot. It adds a new payment command, implements a callback endpoint for payment processing, and sets up a web server to handle API requests. The changes also include new environment variables for Paydisini configuration and modifications to the bot's initialization process. File-Level Changes
Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @yasirarism - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider improving error handling and providing more informative error messages, especially in the payment function. This will aid in debugging and improve user experience.
- The payment integration logic is tightly coupled with the API calls. Consider separating the business logic from the API interaction for better maintainability and testability.
- Add comprehensive unit and integration tests for the new payment feature to ensure reliability and catch potential issues early.
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟡 Security: 3 issues found
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| async def payment(client: Client, message: Message): | ||
| api_url = 'https://api.paydisini.co.id/v1/' | ||
| api_key = PAYDISINI_KEY | ||
| unique_id = f"VIP-{secrets.token_hex(5)}" |
There was a problem hiding this comment.
🚨 suggestion (security): Consider using a longer token for the unique ID
While using secrets.token_hex() is good for security, a 5-byte token might not provide sufficient uniqueness for a payment ID. Consider using a longer token, such as secrets.token_hex(16), to reduce the risk of collisions.
| unique_id = f"VIP-{secrets.token_hex(5)}" | |
| unique_id = f"VIP-{secrets.token_hex(16)}" |
| # if id_ in user_data and user_data[id_].get("is_auth"): | ||
| # return await message.reply("Already Authorized!") | ||
| rget = await fetch.post(api_url, data=params) | ||
| if rget.status_code != 200: |
There was a problem hiding this comment.
suggestion: Enhance error handling and logging
The current error handling is quite basic. Consider implementing more robust error handling and logging. For example, log the specific error message and status code, and provide more informative error messages to the user based on different error scenarios.
if rget.status_code != 200:
error_message = f"API Error: Status code {rget.status_code}"
logging.error(f"{error_message}. Response: {rget.text}")
user_message = "An error occurred while processing your request. Please try again later."
return await message.reply(user_message)
| LOGGER = getLogger(__name__) | ||
| getLogger("fastapi").setLevel(ERROR) | ||
|
|
||
| @api.post("/callback") |
There was a problem hiding this comment.
🚨 suggestion (security): Enhance security measures for payment callback
While you're verifying the IP and signature, consider adding additional security measures such as rate limiting, HTTPS enforcement, and possibly a webhook secret. Also, ensure that all sensitive data is properly sanitized before use.
@api.post("/callback", dependencies=[Depends(RateLimiter(times=10, seconds=60))])
@requires_https
async def autopay(request: Request, signature: str = Header(None)):
if not verify_signature(signature, request):
raise HTTPException(status_code=403, detail="Invalid signature")
| exists = await autopay.find_one({"_id": uniqueCode}) | ||
| return exists | ||
|
|
||
| async def autopay_update(self, msg_id: Optional[int] = "", note: Optional[str] = "", user_id: Optional[int] = "", amount: Optional[int] = "", status: Optional[str] = "", uniqueCode: Optional[str] = "", createdAt: Optional[str] = ""): |
There was a problem hiding this comment.
🚨 suggestion (security): Improve type hints and parameter handling
The type hints for integer fields (msg_id, user_id, amount) default to empty strings, which is inconsistent. Consider using None as the default value for these Optional fields. Also, validate and sanitize these inputs before using them in database operations to prevent potential injection attacks.
async def autopay_update(
self,
uniqueCode: str,
msg_id: Optional[int] = None,
note: Optional[str] = None,
user_id: Optional[int] = None,
amount: Optional[int] = None,
status: Optional[str] = None,
createdAt: Optional[str] = None
):
data = {k: v for k, v in locals().items() if k != 'self' and v is not None}
await autopay.update_one({"_id": uniqueCode}, {"$set": data}, upsert=True)
| @@ -179,6 +184,45 @@ | |||
| else: | |||
| await msg.edit_msg("Unsupported parameter") | |||
|
|
|||
| @app.on_message(filters.command(["payment"], COMMAND_HANDLER)) | |||
There was a problem hiding this comment.
issue (complexity): Consider refactoring the 'payment' function into smaller, focused helper functions.
The payment function introduces significant complexity by handling multiple responsibilities within a single function. To improve readability and maintainability, consider breaking it down into smaller, focused helper functions. Here's a suggested refactoring:
- API Call: Extract the API interaction into a separate function.
async def create_payment(api_url, api_key, unique_id, service_id, amount, valid_time):
params = {
'key': api_key,
'request': 'new',
'unique_code': unique_id,
'service': service_id,
'amount': amount,
'note': 'MissKaty Support by YS Dev',
'valid_time': valid_time,
'type_fee': '1',
'payment_guide': True,
'signature': hashlib.md5((api_key + unique_id + service_id + amount + valid_time + 'NewTransaction').encode()).hexdigest(),
'return_url': f'https://t.me/{client.me.username}?start'
}
return await fetch.post(api_url, data=params)- QR Code Generation: Isolate the QR code generation logic.
def generate_qr_code(data):
return f"https://api.qrserver.com/v1/create-qr-code/?size=300x300&data={quote(data)}"- Database Update: Separate the database update logic.
async def update_payment_db(msg_id, note, user_id, amount, status, unique_code, created_at):
await autopay_update(msg_id, note, user_id, amount, status, unique_code, created_at)By organizing the code in this way, the main payment function will be more concise and focused, improving both readability and maintainability.
| exists = await autopay.find_one({"_id": uniqueCode}) | ||
| return exists |
There was a problem hiding this comment.
suggestion (code-quality): Inline variable that is immediately returned (inline-immediately-returned-variable)
| exists = await autopay.find_one({"_id": uniqueCode}) | |
| return exists | |
| return await autopay.find_one({"_id": uniqueCode}) |
| valid_time = str(5*60) | ||
| service_id = PAYDISINI_CHANNEL_ID | ||
|
|
||
| params = { |
There was a problem hiding this comment.
issue (code-quality): Replace f-string with no interpolated values with string (remove-redundant-fstring)
About This:
PAYDISINI_KEY = api key paydisini
PAYDISINI_CHANNEL_ID = paydisini channel id, default 11
Daftar dulu disini https://paydisini.co.id/Buat-Transaksi/ dan minta whitelist IP VPS agar bisa dipake, maybe soon this feature will be applied to some feature if user abuse my bot
Summary by Sourcery
Integrate Paydisini payment system into the application, allowing users to make payments and receive payment status updates. Add a new FastAPI-based web server to handle payment callbacks and provide system status information. Enhance the application with new environment variables for Paydisini configuration and introduce a WSGI server task.
New Features:
Enhancements: