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.

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

Feat: support managing predefined prompts with editing and replacement" #454

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 5 additions & 4 deletions src/components/PromptModal.vue
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,14 @@
size="x-small"
icon="mdi-pencil"
@click="edit(item.raw)"
v-if="item.raw.index >= 0"
></v-btn>
></v-btn>
<v-btn
flat
size="x-small"
icon="mdi-delete-outline"
@click="deletePrompt(item.raw)"
v-if="item.raw.index >= 0"
></v-btn>
></v-btn>
Copy link
Collaborator

Choose a reason for hiding this comment

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

This change is not related to the topic.

</td>
</tr>
</template>
Expand Down Expand Up @@ -185,7 +184,9 @@ const userPrompts = computed(() => {
});
});
const data = computed(() => {
const defaultPrompts = prompts[language.value].map((prompt) => {
const userPromptsTitles = userPrompts.value.map(d => d.title);
const valid = prompts[language.value].filter(d=>!userPromptsTitles.includes(d.act));
const defaultPrompts = valid.map((prompt) => {
return { title: prompt.act, prompt: prompt.prompt };
});

Expand Down
3 changes: 2 additions & 1 deletion src/store/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,8 @@ export default createStore({
addPrompt.index = state.prompts.push(addPrompt) - 1;
},
editPrompt(state, values) {
const { index } = values;
let { index } = values;
index=index || state.prompts.length
state.prompts[index] = { ...state.prompts[index], ...values };
},
Comment on lines +320 to 323
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Implement title comparison logic to align with PR objectives

There's a discrepancy between the PR objectives and the current implementation of the editPrompt mutation. The PR states that editing a prompt with the same title should replace the original, while a different title should create a new prompt. However, the current implementation doesn't reflect this behavior. To address this:

  1. Implement the title comparison logic as described in the PR objectives. This might involve searching for an existing prompt with the same title before deciding whether to edit or create a new one.

  2. Add safeguards to prevent unintended data loss or duplication. For example, you could add a confirmation step when overwriting an existing prompt.

  3. Consider updating the mutation to handle both editing and creating new prompts based on the title comparison, or split this functionality into separate mutations for clarity.

Here's a suggested implementation that aligns with the PR objectives:

editPrompt(state, values) {
  const { title, ...otherValues } = values;
  const existingIndex = state.prompts.findIndex(prompt => prompt.title === title);

  if (existingIndex !== -1) {
    // If a prompt with the same title exists, update it
    state.prompts[existingIndex] = { ...state.prompts[existingIndex], ...otherValues };
  } else {
    // If no prompt with the same title exists, create a new one
    state.prompts.push({ title, ...otherValues });
  }
},

This implementation ensures that the behavior matches the PR objectives and prevents unintended overwrites or duplications.

deletePrompt(state, values) {
Expand Down