Skip to content

Add success message to engine export#1414

Closed
NaveenRamasamy028 wants to merge 1 commit intodevfrom
feat/ToasterMessage
Closed

Add success message to engine export#1414
NaveenRamasamy028 wants to merge 1 commit intodevfrom
feat/ToasterMessage

Conversation

@NaveenRamasamy028
Copy link
Copy Markdown
Contributor

on click of export in the engine page added success message

@NaveenRamasamy028 NaveenRamasamy028 requested a review from a team as a code owner July 1, 2025 16:12
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jul 1, 2025

@CodiumAI-Agent /describe

@QodoAI-Agent
Copy link
Copy Markdown

Title

Add success message to engine export


User description

on click of export in the engine page added success message


PR Type

Enhancement


Description

  • Add toaster notifications on engine export

  • Handle download promise with success/error callbacks

  • Include type for personalized messages

  • Adjust export loading state unset


Changes diagram

flowchart LR
  A["Click Export"] --> B["monolithStore.runQuery(pixel)"]
  B --> C["monolithStore.download(insightId, output)"]
  C -- "success" --> D["notification('success', `${type} downloaded successfully`)"]
  C -- "error" --> E["notification('error', `${type} download failed`)"]
Loading

Changes walkthrough 📝

Relevant files
Enhancement
EngineHeader.tsx
Add export notifications and type destructuring                   

packages/client/src/components/engine/EngineHeader.tsx

  • Destructure type from useEngine()
  • Chain .download() promise with then/catch
  • Show success and error toaster messages
  • Manage export loading state within callbacks
  • +19/-2   

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @github-actions
    Copy link
    Copy Markdown

    github-actions bot commented Jul 1, 2025

    @CodiumAI-Agent /review

    @github-actions
    Copy link
    Copy Markdown

    github-actions bot commented Jul 1, 2025

    @CodiumAI-Agent /improve

    @QodoAI-Agent
    Copy link
    Copy Markdown

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Loading state

    setExportLoading(false) is called immediately after runQuery, causing the loading indicator to stop before the download completes and duplicating state updates. Consider removing the extra call outside the download promise.

        setExportLoading(false);
    };
    Error handling

    There's no .catch on monolithStore.runQuery, so failures in the initial request won't reset the loading state or surface an error notification. Consider adding error handling for the query promise.

    const exportDB = () => {
        setExportLoading(true);
        const pixel = `META | ExportEngine(engine=["${active.id}"] );`;
    
        monolithStore.runQuery(pixel).then((response) => {
            const output = response.pixelReturn[0].output,
                insightId = response.insightId;
    Undefined errors

    response.errors is accessed without checking if it exists, which could cause a runtime error. Ensure errors is defined or guard against undefined before checking length.

    if (output && response.errors.length === 0) {

    @QodoAI-Agent
    Copy link
    Copy Markdown

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Impact
    Possible issue
    Remove premature loading reset

    Remove the premature setExportLoading(false) call after initiating the runQuery
    promise, since it resets the loading state immediately instead of waiting for the
    download to finish.

    packages/client/src/components/engine/EngineHeader.tsx [105]

    -setExportLoading(false);
    +// remove this line entirely so loading remains true until download handlers run
    Suggestion importance[1-10]: 8

    __

    Why: The extra setExportLoading(false) at line 105 resets the loading state before the download promise resolves, causing incorrect UI feedback and should be removed.

    Medium
    Handle runQuery failures

    Add a .catch handler on the runQuery promise to handle failures, reset loading
    state, and show an error notification if the initial query fails.

    packages/client/src/components/engine/EngineHeader.tsx [82]

    -monolithStore.runQuery(pixel).then((response) => {
    +monolithStore
    +    .runQuery(pixel)
    +    .then((response) => {
    +        // existing success logic...
    +    })
    +    .catch((error) => {
    +        notification.add({ color: 'error', message: `Export failed: ${error.message}` });
    +        setExportLoading(false);
    +    });
    Suggestion importance[1-10]: 7

    __

    Why: Adding a .catch on monolithStore.runQuery is important to surface query errors and reset the loading state, preventing unhandled promise rejections.

    Medium
    Check errors before download

    Validate that response.errors is empty before calling download; if there are errors,
    skip download, show an error notification, and reset the loading state.

    packages/client/src/components/engine/EngineHeader.tsx [86-87]

    -monolithStore
    -    .download(insightId, output)
    +if (response.errors.length === 0) {
    +    monolithStore.download(insightId, output)
    +        .then(() => { /* success handler */ })
    +        .catch(() => { /* error handler */ });
    +} else {
    +    notification.add({ color: 'error', message: `${type} download aborted due to query errors` });
    +    setExportLoading(false);
    +}
    Suggestion importance[1-10]: 5

    __

    Why: Guarding the download call on response.errors helps avoid unnecessary network requests if the query failed, though the PR already checks errors post-download.

    Low

    @NaveenRamasamy028 NaveenRamasamy028 linked an issue Jul 1, 2025 that may be closed by this pull request
    3 tasks
    @NaveenRamasamy028
    Copy link
    Copy Markdown
    Contributor Author

    Changes has been linked to the below PR
    #1437

    @NaveenRamasamy028 NaveenRamasamy028 linked an issue Jul 10, 2025 that may be closed by this pull request
    3 tasks
    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.

    Add success message to database export

    2 participants