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.

Sign up for GitHub

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

Jump to bottom

[MOC-56] waiting until update finishes before showing rec list #70

Merged
merged 10 commits into from
Jun 21, 2024
Merged

[MOC-56] waiting until update finishes before showing rec list #70

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
5274d5c
now waiting til we update the value on storage before opening recordi…
NicoMorenoSirius Jun 21, 2024
68c41b7
run lint
NicoMorenoSirius Jun 21, 2024
97abe78
Merge remote-tracking branch 'origin' into moc-56-demo-list-update
NicoMorenoSirius Jun 21, 2024
fb61a03
merge with main
NicoMorenoSirius Jun 21, 2024
64ff21a
run lint
NicoMorenoSirius Jun 21, 2024
6317e68
undoing changes after pressing stop button
NicoMorenoSirius Jun 21, 2024
807175d
fixed recording list style
NicoMorenoSirius Jun 21, 2024
701f74c
code rabbit reviews
NicoMorenoSirius Jun 21, 2024
3aad6fd
state update fix
NicoMorenoSirius Jun 21, 2024
059e284
run lint
NicoMorenoSirius Jun 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions apps/mocksi-lite/content/ContentApp.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,16 +94,15 @@ function ShadowContentApp({ isOpen, email }: ContentProps) {
<div className="border border-solid border-grey/40 rounded-l bg-white mt-3 min-w-64 p-3 flex flex-row items-center gap-6">
<div
className="cursor-pointer"
onClick={async () => {
onClick={() => {
// We don't pass the recordingId here because we don't want to persist the changes
setEditorMode(false);
onChangeState(RecordingState.CREATE);
const recordingId = await loadRecordingId();
setEditorMode(false, recordingId);
}}
onKeyUp={async (event) => {
onKeyUp={(event) => {
if (event.key === "Escape") {
setEditorMode(false);
onChangeState(RecordingState.CREATE);
const recordingId = await loadRecordingId();
setEditorMode(false, recordingId);
}
}}
>
Expand All @@ -126,15 +125,15 @@ function ShadowContentApp({ isOpen, email }: ContentProps) {
<div
className="cursor-pointer text-[#009875]"
onClick={async () => {
onChangeState(RecordingState.CREATE);
const recordingId = await loadRecordingId();
setEditorMode(false, recordingId);
await setEditorMode(false, recordingId);
onChangeState(RecordingState.CREATE);
}}
onKeyUp={async (event) => {
if (event.key === "Enter") {
onChangeState(RecordingState.CREATE);
const recordingId = await loadRecordingId();
setEditorMode(false, recordingId);
await setEditorMode(false, recordingId);
onChangeState(RecordingState.CREATE);
Copy link

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Unit tests are missing for the onKeyUp event handler in ContentApp.tsx.

To ensure the new logic is thoroughly tested, please add unit tests for the onKeyUp event handler in ContentApp.tsx. This will help verify that setEditorMode and onChangeState are correctly called after loadRecordingId.

  • Add unit tests for the onKeyUp event handler in apps/mocksi-lite/content/ContentApp.tsx.
Analysis chain

Refactor: Modified onKeyUp event handler for "Enter" key.

This change ensures that setEditorMode and onChangeState are called after loadRecordingId, which is crucial for ensuring that the application's state is consistent with the backend data. This is a critical change and should be thoroughly tested.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify if unit tests cover the new logic in onKeyUp handler for "Enter" key.
rg --type typescript -g '*test.ts' 'onKeyUp'

Length of output: 79

Script:

#!/bin/bash
# Correcting the file type and re-verifying if unit tests cover the new logic in onKeyUp handler for "Enter" key.
rg --type ts -g '*test.ts' 'onKeyUp'

Length of output: 36

Script:

#!/bin/bash
# Search for the implementation of the onKeyUp event handler in the codebase.
rg 'onKeyUp' --type ts

# Search for test files to verify if the onKeyUp event handler is covered by tests.
fd -e ts -e tsx -e js -e jsx test

Length of output: 999

}
}}
>
Expand Down
3 changes: 2 additions & 1 deletion apps/mocksi-lite/content/EditMode/editMode.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export const setEditorMode = async (turnOn: boolean, recordingId?: string) => {
return;
}
if (recordingId) {
persistModifications(recordingId);
await persistModifications(recordingId);
}
undoModifications();
await chrome.storage.local.set({
Expand All @@ -31,6 +31,7 @@ export const setEditorMode = async (turnOn: boolean, recordingId?: string) => {
restoreNodes();
cancelEditWithoutChanges(document.getElementById("mocksiSelectedText"));
document.normalize();
return;
};

function onDoubleClickText(event: MouseEvent) {
Expand Down
6 changes: 1 addition & 5 deletions apps/mocksi-lite/content/Popup/CreateDemo/DemoItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,7 @@ const DemoItem = ({
<Button
variant={Variant.icon}
onClick={() => loadAlterations(alterations, false)}
disabled={
!url.includes(window.location.hostname) ||
!alterations ||
!alterations.length
}
disabled={!url.includes(window.location.hostname)}
>
<img src={exportIcon} alt={"exportIcon"} />
</Button>
Expand Down
31 changes: 30 additions & 1 deletion apps/mocksi-lite/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,17 @@ export const saveModification = (
saveModificationCommand.execute();
};

export const persistModifications = (recordingId: string) => {
export const persistModifications = async (recordingId: string) => {
const alterations: Alteration[] = buildAlterations(domainModifications);
chrome.storage.local.set({
[MOCKSI_MODIFICATIONS]: JSON.stringify(domainModifications),
});
const updated_timestamp = new Date();
await updateRecordingsStorage({
uuid: recordingId,
updated_timestamp,
alterations,
});
sendMessage("updateDemo", {
id: recordingId,
recording: { updated_timestamp, alterations },
Expand Down Expand Up @@ -264,3 +269,27 @@ export const getRecordingsStorage = async (): Promise<Recording[]> => {
throw err;
}
};

export const updateRecordingsStorage = async ({
uuid,
updated_timestamp,
alterations,
}: { uuid: string; updated_timestamp: Date; alterations: Alteration[] }) => {
try {
const recordingsFromStorage = await getRecordingsStorage();
const modifiedRecordings = recordingsFromStorage.map((recording) =>
recording.uuid === uuid
? { ...recording, uuid, updated_timestamp, alterations }
: recording,
);
const sorted = modifiedRecordings.sort((a: Recording, b: Recording) =>
a.updated_timestamp > b.updated_timestamp ? -1 : 0,
);
const recordingsStringified = JSON.stringify(sorted);
console.log("modified", recordingsFromStorage, sorted);
chrome.storage.local.set({ recordings: recordingsStringified });
} catch (err) {
console.error("Failed to save modifications from LS:", err);
throw err;
}
};
Comment on lines +282 to +304
Copy link

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Unit tests for updateRecordingsStorage function are missing.

The search results indicate that the updateRecordingsStorage function is defined and used within apps/mocksi-lite/utils.ts, but there are no references to it in any test files. This suggests that the function is not currently covered by unit tests.

  • Ensure that unit tests are written for the updateRecordingsStorage function to verify its behavior under various scenarios.
Analysis chain

New Feature: Added updateRecordingsStorage function to handle storage updates.

This addition is significant as it centralizes the logic for updating recordings in storage, which can improve maintainability and testing. The function also handles errors robustly, which is crucial for reliability. Ensure that this function is covered by unit tests to verify its behavior under various scenarios.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Verify if unit tests cover the new `updateRecordingsStorage` function.
rg --type typescript -g '*test.ts' 'updateRecordingsStorage'

Length of output: 95

Script:

#!/bin/bash
# Verify if unit tests cover the new `updateRecordingsStorage` function.
rg --type ts -g '*test.ts' 'updateRecordingsStorage'

Length of output: 52

Script:

#!/bin/bash
# Find all test files in the repository
fd --type f --extension ts --exec rg 'updateRecordingsStorage' {}

Length of output: 148

Script:

#!/bin/bash
# Find all test files in the repository and their occurrences of `updateRecordingsStorage`
fd --type f --extension ts --exec rg --with-filename --line-number 'updateRecordingsStorage' {}

Length of output: 241