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

chore(formula): defined name ref range move #3488

Merged
merged 18 commits into from
Oct 11, 2024
Merged

Conversation

Dushusir
Copy link
Member

@Dushusir Dushusir commented Sep 19, 2024

优化:

  • 抽离公式更新 ref range 偏移参数和偏移范围函数到工具方法,和 defined name 公用(无需 QA 测试)
  • 求两个范围的交集的方法 Rectangle.getIntersects 未正确处理 NaNrangeType,已新写方法 getIntersectRange,推荐按需替换掉(无需 QA 测试)

功能:

  • defined name 适配 ref range 更新(如何测试:新建 defined name,测试插入行、插入列、删除 sheet 等等对 defined name 范围的影响)
  • 公式适配 defined name 更新(如何测试:公式引用 defined name,修改 defined name 的范围或者名称,检验公式是否也会更新,包括公式提示)

修复:

  • close https://github.com/dream-num/univer-pro/issues/2508
  • 修复 defined name 跳转隐藏 sheet bug(如何测试:defined name 跳转到被隐藏的 sheet,则先显示该 sheet 之后再跳转,参考 Google Sheets,注意与超链接跳转 defined name 不同,超链接会拦截跳转并提示)

Pull Request Checklist

  • Related tickets or issues have been linked in the PR description (or missing issue).
  • Naming convention is followed (do please check it especially when you created new plugins, commands and resources).
  • Unit tests have been added for the changes (if applicable).
  • Breaking changes have been documented (or no breaking changes introduced in this PR).

Copy link

github-actions bot commented Sep 19, 2024

View Deployment

📑 Examples 📚 Storybook
🔗 Preview link 🔗 Preview link

Copy link

github-actions bot commented Sep 19, 2024

Playwright test results

passed  16 passed

Details

stats  16 tests across 8 suites
duration  4 minutes
commit  d357eaf
info  For more information, see full report

Copy link

codecov bot commented Sep 19, 2024

Codecov Report

Attention: Patch coverage is 14.16431% with 606 lines in your changes missing coverage. Please review.

Project coverage is 31.27%. Comparing base (f78afc0) to head (d357eaf).
Report is 1 commits behind head on dev.

Files with missing lines Patch % Lines
...ts-formula/src/controllers/utils/ref-range-move.ts 20.00% 224 Missing ⚠️
...s-formula/src/controllers/utils/ref-range-param.ts 0.00% 157 Missing ⚠️
.../src/controllers/update-defined-name.controller.ts 0.00% 77 Missing ⚠️
...rmula/src/controllers/update-formula.controller.ts 0.00% 31 Missing ⚠️
...formula/src/controllers/utils/ref-range-formula.ts 10.00% 27 Missing ⚠️
...ets-hyper-link-ui/src/services/resolver.service.ts 0.00% 12 Missing ⚠️
...ui/src/views/defined-name/DefinedNameContainer.tsx 0.00% 11 Missing ⚠️
.../src/commands/commands/set-defined-name.command.ts 0.00% 10 Missing ⚠️
...ormula/src/controller/set-dependency.controller.ts 0.00% 9 Missing ⚠️
...formula/src/services/dependency-manager.service.ts 0.00% 9 Missing ⚠️
... and 8 more
Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #3488      +/-   ##
==========================================
+ Coverage   31.25%   31.27%   +0.01%     
==========================================
  Files        2260     2263       +3     
  Lines      117121   117370     +249     
  Branches    25761    25835      +74     
==========================================
+ Hits        36603    36703     +100     
- Misses      80518    80667     +149     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Dushusir Dushusir changed the title chore(formula): ref range move chore(formula): ref range move for defined name Sep 21, 2024
@Dushusir Dushusir changed the title chore(formula): ref range move for defined name chore(formula): defined name ref range move Sep 21, 2024
@Dushusir Dushusir marked this pull request as ready for review September 21, 2024 08:43
@univer-bot
Copy link

univer-bot bot commented Sep 21, 2024

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿

Origin Title: chore(formula): defined name ref range move

Title: chore(formula): defined name ref range move


optimization:

  • Extract the formula to update the ref range offset parameter and offset range function to the tool method, and make it public with defined name (no QA testing required)
  • The method Rectangle.getIntersects that finds the intersection of two ranges does not handle NaN and rangeType correctly. A new method getIntersectRange has been written. It is recommended to replace it as needed (no QA testing required)

Function:

  • defined name adapts to ref range update (how to test: create a new defined name, test the impact of inserting rows, inserting columns, deleting sheets, etc. on the defined name range)
  • Formula adaptation defined name update (how to test: formula refers to defined name, modify the scope or name of defined name, check whether the formula will also be updated, including formula prompts)

repair:

  • close https://github.com/dream-num/univer-pro/issues/2508
  • Fix defined name jump to hidden sheet bug (how to test: defined name jumps to a hidden sheet, first display the sheet and then jump, refer to Google Sheets, note that it is different from hyperlink jump to defined name, The hyperlink will intercept the jump and prompt)

Pull Request Checklist

  • Related tickets or issues have been linked in the PR description (or missing issue).
  • Naming convention is followed (do please check it especially when you created new plugins, commands and resources).
  • Unit tests have been added for the changes (if applicable).
  • Breaking changes have been documented (or no breaking changes introduced in this PR).

@Dushusir Dushusir added the qa:untested This PR is ready to be tested label Sep 21, 2024
@oumomomo
Copy link

oumomomo commented Sep 24, 2024

移动行&列
1、将第一行&列移动到最后一行&列,应用范围预期减少【可以测试】
2024_9_24 14_39_48 video.webm

2、将中间的行&列移动到首行&列,应用范围预期减少,从1:6变成2:6/A:D变成B:D【可以测试】
2024_9_24 14_42_02 video.webm

3、将首行&列移动到范围外,应用范围预期减少,从1:6减少至1:5/A:D变成A:C【可以测试】
image

4、将尾行&尾列移动到范围外,应用范围对应拓展至移动的行&列【可以测试】
image

5、将范围内所有行&列移动至范围外,应用范围跟随行列移动到的位置展示【可以测试】
2024_9_24 15_35_15 video.webm

@univer-bot
Copy link

univer-bot bot commented Sep 24, 2024

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿

Move rows & columns

  1. Move the first row & column to the last row & column, the application scope is expected to be reduced [can be tested]
    2024_9_24 14_39_48 video.webm

  2. Move the middle rows & columns to the first row & column. The application range is expected to be reduced, from 1:6 to 2:6/A:D to B:D [can be tested]
    2024_9_24 14_42_02 video.webm

  3. Move the first row & column outside the range. The application range is expected to be reduced from 1:6 to 1:5/A:D becomes A:C [can be tested]
    image

  4. Move the last row & column out of the range, and the application scope will be expanded to the moved rows & columns [can be tested]
    image

  5. Move all rows & columns within the range outside the range, and the application range will be displayed according to the position where the rows and columns are moved [can be tested]
    2024_9_24 15_35_15 video.webm

@univer-bot univer-bot bot removed the qa:untested This PR is ready to be tested label Sep 24, 2024
@oumomomo
Copy link

oumomomo commented Sep 24, 2024

公式&定义名称
1、删除公式引用的定义名称,计算结果未报错【可以测试】
image

2、定义名称引用表1,表2的公式引用定义名称,删除表1【可以测试】
(1)表2的单元格预期应该只展示错误码不展示公式引用框?
(2)将表1 undo回来计算结果还是展示为错误码,且删除前已有的定义名称再次引用也无法计算出结果
image
2024_9_24 17_08_49 video.webm

3、删除定义名称引用的行列,公式计算结果未报错【可以测试】
image

4、引用定义名称时修改定义范围内的单元格数据为公式,结果计算不对 【依赖问题,有点难度,DEV 已有问题,已经单独提交 issue https://github.com/dream-num/univer-pro/issues/2744】
2024_9_24 18_00_38 video.webm

image

5、定义名称为公式时,公式引用表1的数据,删除表1计算结果未报错【编辑器损坏,也移到 https://github.com/dream-num/univer-pro/issues/2744】
2024_9_24 19_54_42 video.webm

@univer-bot
Copy link

univer-bot bot commented Sep 24, 2024

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿

Formula & definition name

  1. Delete the definition name referenced by the formula, and the calculation result will not report an error [can be tested]
    image

  2. The definition name refers to Table 1. The formula in Table 2 refers to the definition name. Delete Table 1 [can be tested]
    (1) The cells in Table 2 are expected to only display error codes and not formula reference boxes?
    (2) The calculation result after undoing Table 1 is still displayed as an error code, and the result cannot be calculated even if the existing definition name is referenced again before deletion.
    image
    2024_9_24 17_08_49 video.webm

  3. Delete the row and column referenced by the definition name, and the formula calculation result does not report an error [can be tested]
    image

4. When referencing the definition name, the cell data within the definition range is modified into a formula, and the calculation result is incorrect [Dependency issue, a bit difficult, DEV already has a problem, and an issue has been submitted separately https://github.com/dream- num/univer-pro/issues/2744]
2024_9_24 18_00_38 video.webm

image

5. When the name is defined as a formula, the formula refers to the data in Table 1, and no error is reported when deleting the calculation result of Table 1[The editor is damaged and has also been moved to https://github.com/dream-num/univer-pro/ issues/2744]
2024_9_24 19_54_42 video.webm

@oumomomo
Copy link

oumomomo commented Sep 24, 2024

超链接&定义名称

1、删除定义名称引用的行列,在超链接进行跳转时提示子表被删除,感觉文案有点奇怪【可以测试,defined name 如果为 #REF! 统一改成了提示 错误的引用
image

2、将定义名称从范围修改为公式,显示可以跳转但实际点击时提示子表被删除 【无法复测 迁移到 https://github.com/dream-num/univer-pro/issues/2745】
image

2024_9_24.19_45_33.video.webm

@univer-bot
Copy link

univer-bot bot commented Sep 24, 2024

Bot detected the issue body's language is not English, translate it automatically. 👯👭🏻🧑‍🤝‍🧑👫🧑🏿‍🤝‍🧑🏻👩🏾‍🤝‍👨🏿👬🏿

Hyperlinks & Definition Names

  1. Delete the row and column referenced by the defined name. When the hyperlink jumps, it prompts that the sub-table is deleted. The copywriting feels a bit strange [can be tested, if defined name is #REF!, it will be changed to prompt wrong reference]
    image

2. Change the definition name from a range to a formula. The display can jump but when actually clicked, it prompts that the subtable is deleted [Unable to retest and migrate to https://github.com/dream-num/univer-pro/ issues/2745]
image

2024_9_24.19_45_33.video.webm

@dream-num dream-num deleted a comment from univer-bot bot Sep 24, 2024
@Dushusir Dushusir force-pushed the dushusir/define-name-bug branch 3 times, most recently from f86771b to fba8a26 Compare September 29, 2024 02:46
@Dushusir Dushusir added the qa:untested This PR is ready to be tested label Sep 29, 2024
@oumomomo oumomomo added the qa:verified This PR has already by verified by a QA and is considered good enough to be merge label Oct 9, 2024
@univer-bot univer-bot bot removed the qa:untested This PR is ready to be tested label Oct 9, 2024
@Dushusir Dushusir force-pushed the dushusir/define-name-bug branch 6 times, most recently from e6e6e62 to f94bb52 Compare October 11, 2024 07:20
@Dushusir Dushusir merged commit 69ffa5b into dev Oct 11, 2024
9 checks passed
@Dushusir Dushusir deleted the dushusir/define-name-bug branch October 11, 2024 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
qa:verified This PR has already by verified by a QA and is considered good enough to be merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants