-
Notifications
You must be signed in to change notification settings - Fork 22
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
Extend support command to dump log #446 #450
Conversation
✅ Deploy Preview for dazzling-kringle-090742 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
WalkthroughThe updated code introduces cache control enhancements to the website, optimizing content delivery with dynamic cache expiration settings. In the command script, it streamlines the command set by removing the Changes
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
✅ Deploy Preview for terminal7 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/commands.ts (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/commands.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files selected for processing (2)
- aatp/peerbook_webrtc/admin.spec.ts (4 hunks)
- src/commands.ts (2 hunks)
Additional comments: 3
src/commands.ts (3)
715-715: The
loginCMD
function is called with anawait
keyword, which is correct since the function is asynchronous. However, ensure that theloginCMD
function is properly handling exceptions to avoid unhandled promise rejections.984-1014: The
supportCMD
function has been enhanced with new functionality for error reporting. Ensure that the server endpoint${schema}://${terminal7.conf.net.peerbook}/support
is correctly configured to handle incoming POST requests with the expected JSON payload. Additionally, consider adding more detailed error messages based on the server's response to aid in troubleshooting.1016-1017: The
loginCMD
function is mentioned in the context of thesubscribeCMD
function. Ensure that theloginCMD
function is updated to handle the new subscription logic and that it properly manages the user's login state.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I found a few minor thing and a major design bug: Now the user has no place to tell us what happened.
You'll also need to add aatp tests to test the front and back and ensure an email is sent. We're already doing something similar - validating an email was sent - in the last function in admin.spec.ts .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files ignored due to filter (1)
- netlify.toml
Files selected for processing (2)
- aatp/peerbook_webrtc/admin.spec.ts (4 hunks)
- src/commands.ts (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- aatp/peerbook_webrtc/admin.spec.ts
- src/commands.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/commands.ts (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/commands.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/commands.ts (3 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/commands.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- aatp/peerbook_webrtc/admin.spec.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- aatp/peerbook_webrtc/admin.spec.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (1)
- aatp/peerbook_webrtc/admin.spec.ts (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- aatp/peerbook_webrtc/admin.spec.ts
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- aatp/peerbook_webrtc/admin.spec.ts (1 hunks)
- src/commands.ts (3 hunks)
Files skipped from review as they are similar to previous changes (2)
- aatp/peerbook_webrtc/admin.spec.ts
- src/commands.ts
It's doesn't run locally , I have a problem with peerbook maybe I'm missing the args , I don't remember them. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
almost there...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/commands.ts (3 hunks)
Additional comments: 1
src/commands.ts (1)
- 14-14: The imports from
@capacitor/filesystem
are correctly added to support file operations within thesupportCMD
function.
The newlines I saw are no longer there. It's strange, maybe something I'll first release PeerBook so the tests will pass and then merge this. Thanks for this, I can't wait for it to delpoyed. I'm seening a bug and |
We can do this together if you want |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- src/commands.ts (3 hunks)
Additional comments: 5
src/commands.ts (5)
- 14-14: The addition of the
Filesystem
,Directory
, andEncoding
imports from@capacitor/filesystem
is appropriate for file operations within thesupportCMD
function.- 991-995: The previous comment regarding the email validation regex is still valid. A more robust solution or a library for email validation should be considered.
- 1006-1007: The previous comment about the conditional operation not assigning the result back to
fieldsNative
is still valid. The suggested fix should be implemented.- 1054-1055: The previous comment about the missing
break
statement in the "Save log to file" case is still valid. The suggested fix should be implemented.- 1057-1057: The previous comment about the
loginCMD
function awaiting itself is still valid. This should be corrected to prevent a recursive call and potential stack overflow error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (2)
- aatp/infra/redis/Dockerfile (1 hunks)
- aatp/peerbook_webrtc/admin.spec.ts (1 hunks)
Additional comments: 1
aatp/infra/redis/Dockerfile (1)
- 2-3: The changes to the Redis configuration settings look correct. Please ensure that disabling AOF persistence (
appendonly no
) and RDB snapshots (save ""
) is intentional and verify that these settings are correctly applied when the container starts.
test('test the support command', async ({request}) => { | ||
await sleep(100) | ||
await page.keyboard.type('support') | ||
await page.keyboard.press('Enter') | ||
await sleep(100) | ||
let twr = await getTWRBuffer(page) | ||
expect(twr).toMatch(/address:$/) | ||
await page.keyboard.type('[email protected]') | ||
await page.keyboard.press('Enter') | ||
await sleep(100) | ||
await page.keyboard.press('ArrowDown') | ||
await page.evaluate(() => { terminal7.log('log line')}) | ||
await page.keyboard.press('Enter') | ||
await sleep(100) | ||
twr = await getTWRBuffer(page) | ||
expect(twr).toMatch(/issue:$/) | ||
await page.keyboard.type('test issue') | ||
await page.keyboard.press('Enter') | ||
|
||
let count = 0 | ||
let msg | ||
while (count < 2) { | ||
await sleep(100) | ||
const res = await request.get('http://smtp:8025/api/v2/messages') | ||
msg = await res.json() | ||
count = msg.count | ||
} | ||
expect(msg.count).toBe(2) | ||
const body = msg.items[0].Content.Body | ||
expect(body).toMatch(/test issue/) | ||
expect(body).toMatch(/log line/) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new test for the "support" command is well-structured and covers the expected behavior. However, consider adding assertions to verify that the email address ([email protected]
) and the log line (log line
) are included in the email body sent by the support command to ensure the complete log dump is captured.
+ expect(body).toMatch(/[email protected]/)
+ expect(body).toMatch(/log line/)
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.
test('test the support command', async ({request}) => { | |
await sleep(100) | |
await page.keyboard.type('support') | |
await page.keyboard.press('Enter') | |
await sleep(100) | |
let twr = await getTWRBuffer(page) | |
expect(twr).toMatch(/address:$/) | |
await page.keyboard.type('[email protected]') | |
await page.keyboard.press('Enter') | |
await sleep(100) | |
await page.keyboard.press('ArrowDown') | |
await page.evaluate(() => { terminal7.log('log line')}) | |
await page.keyboard.press('Enter') | |
await sleep(100) | |
twr = await getTWRBuffer(page) | |
expect(twr).toMatch(/issue:$/) | |
await page.keyboard.type('test issue') | |
await page.keyboard.press('Enter') | |
let count = 0 | |
let msg | |
while (count < 2) { | |
await sleep(100) | |
const res = await request.get('http://smtp:8025/api/v2/messages') | |
msg = await res.json() | |
count = msg.count | |
} | |
expect(msg.count).toBe(2) | |
const body = msg.items[0].Content.Body | |
expect(body).toMatch(/test issue/) | |
expect(body).toMatch(/log line/) | |
}) | |
test('test the support command', async ({request}) => { | |
await sleep(100) | |
await page.keyboard.type('support') | |
await page.keyboard.press('Enter') | |
await sleep(100) | |
let twr = await getTWRBuffer(page) | |
expect(twr).toMatch(/address:$/) | |
await page.keyboard.type('[email protected]') | |
await page.keyboard.press('Enter') | |
await sleep(100) | |
await page.keyboard.press('ArrowDown') | |
await page.evaluate(() => { terminal7.log('log line')}) | |
await page.keyboard.press('Enter') | |
await sleep(100) | |
twr = await getTWRBuffer(page) | |
expect(twr).toMatch(/issue:$/) | |
await page.keyboard.type('test issue') | |
await page.keyboard.press('Enter') | |
let count = 0 | |
let msg | |
while (count < 2) { | |
await sleep(100) | |
const res = await request.get('http://smtp:8025/api/v2/messages') | |
msg = await res.json() | |
count = msg.count | |
} | |
expect(msg.count).toBe(2) | |
const body = msg.items[0].Content.Body | |
expect(body).toMatch(/test issue/) | |
expect(body).toMatch(/log line/) | |
expect(body).toMatch(/test@gmail.com/) | |
expect(body).toMatch(/log line/) | |
}) |
Please review and tell me if there is anything i need to change
Summary by CodeRabbit
New Features
Bug Fixes
login
command to streamline user experience.supportCMD
function for better user interaction and choice handling.Refactor