fix: After modifying the dialogue prefix, the embedded page cannot have a conversation

[shaohuzhang1]

fix: After modifying the dialogue prefix, the embedded page cannot have a conversation

[f2c-ci-robot]

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[f2c-ci-robot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[shaohuzhang1]

The provided code has a few suggestions and considerations:

1. Prefix Parameter: The code now includes an optional prefix parameter to handle different paths within the domain, which is good approach.

2. Template Literals: It's recommended to use template literals for string interpolation where possible, but you're already using them correctly.

3. Dynamic Content Handling: Ensure that the dynamic content (like protocol, host, token, query) provided by your application doesn't contain sensitive information like tokens or passwords directly in the HTML string. Consider passing these through JavaScript variables instead of hardcoding them.

4. Function Calls with Template Strings: When calling functions with multiple parameters using template strings, ensure consistency in spacing around commas.

Here’s a revised version of the code with comments and minor improvements:

const chatButtonHtml = `
<div id="maxkb-chat-container">
  <iframe id="maxkb-chat" allow="microphone"
          src="${protocol}:${host}${prefix}/chat/${token}?mode=embed${encodeURIComponent(query)}"></iframe>
  <div class="maxkb-operate">
    <div class="maxkb-closeviewport maxkb-viewportnone">
      <svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" viewBox="0 0 20 20" fill="none">
        <!-- SVG path details -->
      </svg>
    </div>
  </div>
</div>
`;

function initChat(root) {
  // Add chat button
  root.insertAdjacentHTML("beforeend", chatButtonHtml);

  // Add chat container
  const fullPath = `${protocol}:${host}${prefix}`;
  root.insertAdjacentHTML(
    'beforeend',
    getChatContainerHtml(fullPath, token, { query }, prefix)
  );

  // Button elements
  const chat_button = root.querySelector('.maxkb-chat-button');
  const chat_button_img = root.querySelector('.maxkb-chat-button > img');
}

// Function signature should match expectations
function getChatContainerHtml(protocol, host, params, prefix) {
  return `
  <div id="maxkb-chat-container">
    <iframe id="maxkb-chat" allow="microphone"
            src="${protocol}:${host}${prefix}/chat/${params.token}?mode=embed&${new URLSearchParams(params.query).toString()}"></iframe>
    <div class="maxkb-operate">
      <div class="maxkb-closeviewport maxkb-viewportnone">
        <svg xmlns="http://www.w3.org/2000/svg" width="20" height="20" viewBox="0 0 20 20" fill="none">
          <!-- SVG path details -->
        </svg>
      </div>
    </div>
  </div>`;
}

Key Points:

• Dynamically build the iframe source using template literals.
• Use URLSearchParams to encode the query parameters without adding spaces around commas.
• Pass fullDomain, token, { query }, and prefix as separate arguments to avoid hardcoding unnecessary values in templates.

[shaohuzhang1]

The provided code snippet includes an addition to the get_embed method in the EmbedBuilder class. Specifically, it adds a new parameter prefix which assigns the result of CONFIG.get_chat_path() to this new key 'prefix'. Additionally, there's no change made to existing parameters.

Potential Issues:

1. Unused Parameter: The variable with_valid is declared but not used within the method.

   'white_active': 'true' if application_access_token.white_active else 'false'

2. Lack of Type Specification: It’s good practice to explicitly specify types of function arguments and return values for better readability and maintainability.

3. Uninitialized Variable Reference (application_access_token.white_active) - Ensure application_access_token has been previously defined or properly initialized before use.

4. Magic Number for 'right':

   x_type = location.get('x', {}).get('type', 'right')

   Instead of using hardcoded strings (like "'right'" without quotes), consider defining constants for these kinds of configurations:

RIGHT_ALIGN = "right"
LEFT_ALIGN = "left"

Then update your logic accordingly:

x_type = location.get("x", {}).get("type", RIGHT_ALIGN)  # Assuming right is the default alignment

Suggested Optimizations / Improvements:

• Remove unused imports/declarations that might not be needed elsewhere.

• Add type annotations to functions parameters & return values, e.g.,

  def get_embed(
      self,
      with_valid: bool = True,
      params: Optional[dict] = None
  ) -> dict:
      ...
      return {
          ...
      }

• Use more descriptive names instead of single-character placeholders like "q" for 'query'.

Overall, the code seems mostly clean and functional, but incorporating the above mentioned improvements would make it both clearer and robust.