Debug by roares · Pull Request #1 · oss-know/superset

roares · 2022-11-09T03:37:31Z

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

crystaldust · 2022-11-09T08:47:52Z

 };

-export const TableSelectorMultiple: FunctionComponent<TableSelectorProps> =
+export const TableSelectorMultiple: FunctionComponent<TableSelectorProps>=


此处删掉了空格（应该保留），最好在IDE中配置好prettier，在Save文件时，自动执行prettier进行代码格式化，避免手工维护代码格式

好的,已修改.

crystaldust · 2022-11-09T08:49:15Z

+    ];
+    setTemplatesInfo(templatesInfo);
+    // 获取templates
+    // 赋值


对于语义已经很明显的代码，不必再额外写注释说明（100行）

删掉了多余的注释.

crystaldust · 2022-11-09T08:53:12Z

+
+  // 触发template切换事件，然后这里要替换currentTemplate
+  function changeTemplate(value) {
+    if (value) {


这里有一个潜在的陷阱，如果template id从0开始编号， if (0) 会判定为false。
最好和后端接口开发人员限定好，value用字符串类型，并且具备一定的语义，如"COMPANY_CONTRIBUTION_DISTRIBUTION"

这里我参数命名错误,value实际上是一个object,
类似 {key: ...,label:...,value: ...}

crystaldust · 2022-11-09T08:56:06Z

+    );
+  }
+
+  function renderParamsInput(templateParams) {


这里的变量命名注意名词的单复数，根据169行，传入的是"某一个模板参数"，这里用复数容易引起误解，以为是一个数组类型，用templateParam即可

好的,已修改

…ponent

crystaldust · 2022-11-21T10:52:55Z

      schema: activeQueryEditor ? activeQueryEditor.schema : null,
      autorun: false,
-      sql: `${warning}SELECT ...`,
+      sql: `${warning}SELECT ...。。。。。ss`,


这部分提示信息不是必需的

crystaldust · 2022-11-21T10:58:19Z

@@ -0,0 +1,51 @@
+/**


src/components目录下新增的几个组件，看上去功能和代码量都不大，可以考虑直接写成components/AddLabel.tsx这样的文件即可。components/COMPONENT_NAME/index.tsx这种结构，一般会用于大型的组件，大型组件中为了完成相关功能，又包含很多小型组件，最终结构大概会变成：

components/index.tsx

components/sub_component_1.tsx

components/sub_component_2.tsx

components/utils.ts

因为单个子组件或者工具脚本要单独存放在代码文件中，才会创建component/COMPONENT_NAME/这个目录

crystaldust · 2022-11-21T11:00:30Z

@@ -0,0 +1,50 @@
+/**


此外，AddLabel，DeleteLabel的命名非常通用化，字面意思似乎是“添加Label”，“删除Label”，无法直接理解其含义。可以考虑用具体一些的名称，或者能够准确表达组件功能的名称。

crystaldust · 2022-11-21T11:01:49Z


+  const [buttonLoading, setButtonLoading] = useState(false);
+  const [params, setParams] = useState({});
+  const [template_id, setTemplateId] = useState('');


变量的命名尽量统一，在JavaScript/TypeScript里一般推荐驼峰命名，python和http接口一般用下划线风格

crystaldust · 2022-11-21T11:02:25Z

+
+  function postTemplateParamsData(payload: object) {
+    return SupersetClient.post({
+      url: 'http://192.168.8.60:5000/api/dataset',


一般访问地址不能写死在代码里，通过环境变量、配置文件等方式传入

crystaldust · 2022-11-21T11:03:25Z

+      .catch(e => {
+        setButtonLoading(false);
+        console.log('error', e);
+        // 这里应该改成错误提示信息弹出框


如果是待完成的功能，可以在注释开头增加TODO字样，这样现代IDE会自动分析，可以看到所有代办的任务：

// TODO Do something in the future

crystaldust · 2022-11-21T11:04:37Z

+      </>
+    );
+  }
+  function DatasetNameChange(value: string) {


函数命名风格统一，保持驼峰命名

crystaldust · 2022-11-21T11:06:03Z


-export const TableSelectorMultiple: FunctionComponent<TableSelectorProps> =
-  props => <TableSelector tableSelectMode="multiple" {...props} />;
+export const TableSelectorMultiple: FunctionComponent<


这部分代码没有实质变化，原代码的风格是推荐的风格

crystaldust · 2022-11-21T11:11:03Z

+
+  useEffect(() => {
+    if (currentTemplate) {
+      const par = {};


par命名过于简略，容易产生多种含以上的猜测，用param即可

crystaldust · 2022-11-21T11:15:06Z

+  }
+
+  function renderParamsInput(templateParam) {
+    return templateParam.description.indexOf('json数组') === -1


这个判定条件有点不稳定，需要前后端一起确认一种比较可靠的校验方式

YuXia and others added 3 commits November 8, 2022 10:21

debug

aea90be

Constraint select option properties to [lable, value] to make it work

2410cbb

Added run button without event binding

00b4cdd

crystaldust requested changes Nov 9, 2022

View reviewed changes

roares added 6 commits November 10, 2022 11:29

Adjusted the code format

fe22ee7

Add post request

b2b0185

Replaces the SqlEditorLeftBar component with the TemplateSelector com…

c08a2f1

…ponent

接口不通

3aa2bc9

Implemented page jumping

d85df60

Update templateselector key

a868b34

crystaldust reviewed Nov 21, 2022

View reviewed changes

增加了表单验证，但是还不完善

eef78c9

Conversation

roares commented Nov 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

crystaldust Nov 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

roares commented Nov 9, 2022 •

edited

Loading

crystaldust Nov 9, 2022 •

edited

Loading