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

edit-config button #108

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

edit-config button #108

wants to merge 2 commits into from

Conversation

Cedarich
Copy link

@Cedarich Cedarich commented Feb 17, 2025

This PR introduces the ability to edit existing Starky role configurations through a guided flow with pre-filled values, improving user experience and reducing configuration errors.

Implementation Checklist ✅

  • Added dynamic edit button with config-specific ID
  • Implemented pre-filled value propagation through setup flow
  • Modified configuration steps to display previous values inline
  • Updated database operations to overwrite existing records
  • Added session management for concurrent edits

Testing Protocol 🔍

  1. Duplicate Config Detection
    /add-config @ExistingRole → Verify edit button appears

  2. Edit Flow Validation

    • Click edit button
    • Verify network selection shows current value
    • Change module settings → Confirm summary shows old→new values
  3. Concurrency Test
    Two users editing same config → Last valid submit wins

Notes 📌

  • Maintains backward compatibility with existing configs
  • Adds session timeout (15min inactivity)

Copy link
Collaborator

@Marchand-Nicolas Marchand-Nicolas left a comment

Choose a reason for hiding this comment

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

Have you tested your code ? I get this error
Screenshot 2025-02-19 at 10 33 37 AM

@Marchand-Nicolas
Copy link
Collaborator

Also, CONFIG_TIMEOUT is unused, so I guess you can remove it

@Cedarich
Copy link
Author

Have you tested your code ? I get this error Screenshot 2025-02-19 at 10 33 37 AM

@Cedarich Cedarich closed this Feb 19, 2025
@Cedarich
Copy link
Author

@Marchand-Nicolas I have made the requested changes, including removing the unused CONFIG_TIMEOUT variable and fixing the reported error. I have tested the code, and it is working as expected. Please let me know if you need any more updates. Thank you!

@Cedarich Cedarich reopened this Feb 19, 2025
Copy link
Collaborator

@Marchand-Nicolas Marchand-Nicolas left a comment

Choose a reason for hiding this comment

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

image

I still have the same problem.
It is printed in discord/interactions/index.ts, line 273

@@ -64,7 +63,7 @@ const NETWORK_OPTIONS = [

const SUPPORTED_NETWORKS = ["mainnet", "sepolia"] as const;

const addBackButton = (previousStepId: string) => {
export const handleBackButton = (previousStepId: string) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I get

Type '(previousStepId: string) => ActionRowBuilder<ButtonBuilder>' is not assignable to type 'ChatInputHandler | ButtonHandler | SelectMenuHandler | ModalSubmitHandler'.
  Type '(previousStepId: string) => ActionRowBuilder<ButtonBuilder>' is not assignable to type 'ChatInputHandler'.
    Types of parameters 'previousStepId' and 'interaction' are incompatible.
      Type 'ChatInputCommandInteraction<CacheType>' is not assignable to type 'string'.ts(2322)
index.ts(77, 3): The expected type comes from property 'handler' which is declared here on type 'HandlerConfig'

discord/interactions/index.ts line 194

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants