[MOC-203] mocksi-lite-next authentication #152
Conversation
WalkthroughWalkthroughThe changes enhance the functionality of the background and content scripts within the extension. The background script introduces an authentication retrieval process and manages external messages, while the content script adds a listener for storage changes and expands message handling related to iframe states. These modifications improve the interactivity and user experience of the extension. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant BackgroundScript
participant Storage
participant ContentScript
User ->> BackgroundScript: Install Extension
BackgroundScript -->> User: Open URL for onboarding
User ->> ContentScript: Send STORAGE_CHANGE_EVENT
ContentScript ->> Storage: Update Local Storage
Storage -->> ContentScript: Confirm Update
ContentScript -->> User: Log Update Confirmation
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 Configuration File (
|
fitzk
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (1)
apps/mocksi-lite-next/src/pages/background/index.ts (1)
43-73: Add comments for clarity in message handling.Consider adding comments to explain the purpose of each conditional branch for better readability.
Apply this diff to add comments:
if (request.message === "UNAUTHORIZED") { + // Handle unauthorized requests by retrieving authentication details const auth = await getAuth(); if (auth) { const { accessToken, email } = auth; sendResponse({ message: { accessToken, email }, status: "ok", }); } else { + // Open authentication URL if auth details are not found chrome.tabs.create({ url: "https://nest-auth-ts-merge.onrender.com", }); sendResponse({ message: "authenticating", status: "ok", }); } } else { + // Handle other messages by querying active tabs chrome.tabs.query({ active: true, currentWindow: true }, function (tabs) { if (tabs[0].id) { chrome.tabs.sendMessage(tabs[0].id, { message: request.message }); } else { console.log("No active tab found, could not send message"); } }); sendResponse({ message: request.message, status: "ok" }); return true; }
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- apps/mocksi-lite-next/src/pages/background/index.ts (2 hunks)
- apps/mocksi-lite-next/src/pages/content/mocksi-extension.tsx (2 hunks)
Additional comments not posted (1)
apps/mocksi-lite-next/src/pages/background/index.ts (1)
18-23: Verify cross-browser compatibility for the "install" event.Ensure that the
chrome.tabs.createfunction works as expected on browsers other than Chrome.Run the following script to check for compatibility issues:
| console.log("background script loaded"); | ||
|
|
||
| const getAuth = async (): Promise<null | { | ||
| accessToken: string; | ||
| email: string; | ||
| }> => { | ||
| const MOCKSI_AUTH = "mocksi-auth"; | ||
| try { | ||
| const storageAuth = await chrome.storage.local.get(MOCKSI_AUTH); | ||
| const mocksiAuth = JSON.parse(storageAuth[MOCKSI_AUTH]); | ||
| return mocksiAuth; | ||
| } catch (err) { | ||
| console.error(err); | ||
| return null; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Improve error handling in getAuth.
Consider providing more context in the error message to aid debugging.
Apply this diff to improve error handling:
} catch (err) {
- console.error(err);
+ console.error("Error retrieving authentication from local storage:", err);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.
| console.log("background script loaded"); | |
| const getAuth = async (): Promise<null | { | |
| accessToken: string; | |
| email: string; | |
| }> => { | |
| const MOCKSI_AUTH = "mocksi-auth"; | |
| try { | |
| const storageAuth = await chrome.storage.local.get(MOCKSI_AUTH); | |
| const mocksiAuth = JSON.parse(storageAuth[MOCKSI_AUTH]); | |
| return mocksiAuth; | |
| } catch (err) { | |
| console.error(err); | |
| return null; | |
| } | |
| }; | |
| console.log("background script loaded"); | |
| const getAuth = async (): Promise<null | { | |
| accessToken: string; | |
| email: string; | |
| }> => { | |
| const MOCKSI_AUTH = "mocksi-auth"; | |
| try { | |
| const storageAuth = await chrome.storage.local.get(MOCKSI_AUTH); | |
| const mocksiAuth = JSON.parse(storageAuth[MOCKSI_AUTH]); | |
| return mocksiAuth; | |
| } catch (err) { | |
| console.error("Error retrieving authentication from local storage:", err); | |
| return null; | |
| } | |
| }; |
| const STORAGE_CHANGE_EVENT = "MOCKSI_STORAGE_CHANGE"; | ||
|
|
||
| const div = document.createElement("div"); | ||
| div.id = "__root"; | ||
| document.body.appendChild(div); | ||
| let mounted = false; | ||
| const reactor = new Reactor(); | ||
|
|
||
| window.addEventListener("message", (event: MessageEvent) => { | ||
| const eventData = event.data; | ||
| if (event.source !== window || !eventData || !eventData.type) { | ||
| return; | ||
| } | ||
|
|
||
| if (eventData.type.toUpperCase() === STORAGE_CHANGE_EVENT.toUpperCase()) { | ||
| chrome.storage.local.set({ [eventData.key]: eventData.value }).then(() => { | ||
| console.debug(eventData.key, " set."); | ||
| }); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Add type checks for event data in the message listener.
Ensure eventData.key and eventData.value are of expected types before using them to update local storage.
Apply this diff to add type checks:
if (eventData.type.toUpperCase() === STORAGE_CHANGE_EVENT.toUpperCase()) {
+ if (typeof eventData.key === "string" && typeof eventData.value !== "undefined") {
chrome.storage.local.set({ [eventData.key]: eventData.value }).then(() => {
console.debug(eventData.key, " set.");
});
+ } else {
+ console.error("Invalid event data:", eventData);
+ }
}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.
| const STORAGE_CHANGE_EVENT = "MOCKSI_STORAGE_CHANGE"; | |
| const div = document.createElement("div"); | |
| div.id = "__root"; | |
| document.body.appendChild(div); | |
| let mounted = false; | |
| const reactor = new Reactor(); | |
| window.addEventListener("message", (event: MessageEvent) => { | |
| const eventData = event.data; | |
| if (event.source !== window || !eventData || !eventData.type) { | |
| return; | |
| } | |
| if (eventData.type.toUpperCase() === STORAGE_CHANGE_EVENT.toUpperCase()) { | |
| chrome.storage.local.set({ [eventData.key]: eventData.value }).then(() => { | |
| console.debug(eventData.key, " set."); | |
| }); | |
| } | |
| }); | |
| const STORAGE_CHANGE_EVENT = "MOCKSI_STORAGE_CHANGE"; | |
| const div = document.createElement("div"); | |
| div.id = "__root"; | |
| document.body.appendChild(div); | |
| let mounted = false; | |
| const reactor = new Reactor(); | |
| window.addEventListener("message", (event: MessageEvent) => { | |
| const eventData = event.data; | |
| if (event.source !== window || !eventData || !eventData.type) { | |
| return; | |
| } | |
| if (eventData.type.toUpperCase() === STORAGE_CHANGE_EVENT.toUpperCase()) { | |
| if (typeof eventData.key === "string" && typeof eventData.value !== "undefined") { | |
| chrome.storage.local.set({ [eventData.key]: eventData.value }).then(() => { | |
| console.debug(eventData.key, " set."); | |
| }); | |
| } else { | |
| console.error("Invalid event data:", eventData); | |
| } | |
| } | |
| }); |
| // reactor | ||
| if (request.message === "EDITING") { | ||
| reactor.attach(document, getHighlighter()); | ||
| } | ||
| if (request.message === "STOP_EDITING") { | ||
| reactor.detach(); | ||
| } | ||
| // resize iframe | ||
| if (iframeRef.current) { | ||
| let styles = {}; | ||
| switch (request.message) { | ||
| case "ANALYZING": | ||
| case "EDITING": | ||
| case "PLAY": | ||
| case "RECORDING": |
There was a problem hiding this comment.
Extract style objects into constants for better readability.
Consider defining the style objects as constants outside the switch statement to improve readability and maintainability.
Apply this diff to extract style objects:
+const STYLES = {
+ ANALYZING: { bottom: "auto", height: "150px", top: "0px", width: "400px" },
+ MINIMIZED: { bottom: "10px", height: "100px", top: "auto", width: "100px" },
+ DEFAULT: { bottom: "10px", height: "600px", top: "auto", width: "500px" },
+};
...
switch (request.message) {
case "ANALYZING":
case "EDITING":
case "PLAY":
case "RECORDING":
- styles = {
- bottom: "auto",
- height: "150px",
- top: "0px",
- width: "400px",
- };
+ styles = STYLES.ANALYZING;
break;
case "MINIMIZED":
- styles = {
- bottom: "10px",
- height: "100px",
- top: "auto",
- width: "100px",
- };
+ styles = STYLES.MINIMIZED;
break;
default:
- styles = {
- bottom: "10px",
- height: "600px",
- top: "auto",
- width: "500px",
- };
+ styles = STYLES.DEFAULT;
}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.
| // reactor | |
| if (request.message === "EDITING") { | |
| reactor.attach(document, getHighlighter()); | |
| } | |
| if (request.message === "STOP_EDITING") { | |
| reactor.detach(); | |
| } | |
| // resize iframe | |
| if (iframeRef.current) { | |
| let styles = {}; | |
| switch (request.message) { | |
| case "ANALYZING": | |
| case "EDITING": | |
| case "PLAY": | |
| case "RECORDING": | |
| // reactor | |
| if (request.message === "EDITING") { | |
| reactor.attach(document, getHighlighter()); | |
| } | |
| if (request.message === "STOP_EDITING") { | |
| reactor.detach(); | |
| } | |
| // resize iframe | |
| if (iframeRef.current) { | |
| const STYLES = { | |
| ANALYZING: { bottom: "auto", height: "150px", top: "0px", width: "400px" }, | |
| MINIMIZED: { bottom: "10px", height: "100px", top: "auto", width: "100px" }, | |
| DEFAULT: { bottom: "10px", height: "600px", top: "auto", width: "500px" }, | |
| }; | |
| let styles = {}; | |
| switch (request.message) { | |
| case "ANALYZING": | |
| case "EDITING": | |
| case "PLAY": | |
| case "RECORDING": | |
| styles = STYLES.ANALYZING; | |
| break; | |
| case "MINIMIZED": | |
| styles = STYLES.MINIMIZED; | |
| break; | |
| default: | |
| styles = STYLES.DEFAULT; | |
| } |
| } else { | ||
| console.log("No active tab found, could not send message"); | ||
| chrome.tabs.create({ | ||
| url: "https://nest-auth-ts-merge.onrender.com", |
There was a problem hiding this comment.
Let's change this to use env variables so we can easily switch between local and production
dep PR: https://github.com/Mocksi/nest/pull/21
Summary by CodeRabbit
New Features
Bug Fixes