Skip to content
Merged
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
1 change: 1 addition & 0 deletions apps/chat/serializers/chat_embed_serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ def get_embed(self, with_valid=True, params=None):
'white_active': 'true' if application_access_token.white_active else 'false',
'is_draggable': is_draggable,
'float_icon': float_icon,
'prefix': CONFIG.get_chat_path(),
'query': query,
'show_guide': show_guide,
'x_type': float_location.get('x', {}).get('type', 'right'),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Expand Down
6 changes: 3 additions & 3 deletions apps/chat/template/embed.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ const chatButtonHtml=



const getChatContainerHtml=(protocol,host,token,query)=>{
const getChatContainerHtml=(protocol,host,token,query,prefix)=>{
return `<div id="maxkb-chat-container">
<iframe id="maxkb-chat" allow="microphone" src=${protocol}://${host}/chat/${token}?mode=embed${query}></iframe>
<iframe id="maxkb-chat" allow="microphone" src=${protocol}://${host}${prefix}/${token}?mode=embed${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">
<path d="M7.507 11.6645C7.73712 11.6645 7.94545 11.7578 8.09625 11.9086C8.24706 12.0594 8.34033 12.2677 8.34033 12.4978V16.7976C8.34033 17.0277 8.15378 17.2143 7.92366 17.2143H7.09033C6.86021 17.2143 6.67366 17.0277 6.67366 16.7976V14.5812L3.41075 17.843C3.24803 18.0057 2.98421 18.0057 2.82149 17.843L2.23224 17.2537C2.06952 17.091 2.06952 16.8272 2.23224 16.6645L5.56668 13.3311H3.19634C2.96622 13.3311 2.77967 13.1446 2.77967 12.9145V12.0811C2.77967 11.851 2.96622 11.6645 3.19634 11.6645H7.507ZM16.5991 2.1572C16.7619 1.99448 17.0257 1.99448 17.1884 2.1572L17.7777 2.74645C17.9404 2.90917 17.9404 3.17299 17.7777 3.33571L14.4432 6.66904H16.8136C17.0437 6.66904 17.2302 6.85559 17.2302 7.08571V7.91904C17.2302 8.14916 17.0437 8.33571 16.8136 8.33571H12.5029C12.2728 8.33571 12.0644 8.24243 11.9136 8.09163C11.7628 7.94082 11.6696 7.73249 11.6696 7.50237V3.20257C11.6696 2.97245 11.8561 2.7859 12.0862 2.7859H12.9196C13.1497 2.7859 13.3362 2.97245 13.3362 3.20257V5.419L16.5991 2.1572Z" fill="{{header_font_color}}"/>
</svg></div>
Expand Down Expand Up @@ -62,7 +62,7 @@ const initChat=(root)=>{
// 添加对话icon
root.insertAdjacentHTML("beforeend",chatButtonHtml)
// 添加对话框
root.insertAdjacentHTML('beforeend',getChatContainerHtml('{{protocol}}','{{host}}','{{token}}','{{query}}'))
root.insertAdjacentHTML('beforeend',getChatContainerHtml('{{protocol}}','{{host}}','{{token}}','{{query}}','{{prefix}}'))
// 按钮元素
const chat_button=root.querySelector('.maxkb-chat-button')
const chat_button_img=root.querySelector('.maxkb-chat-button > img')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Expand Down
Loading