appsmith icon indicating copy to clipboard operation
appsmith copied to clipboard

feat: clean up datasource review page

Open jacquesikot opened this issue 1 year ago • 4 comments

Description

Remove the "Generate new page" button from the datasource review page and make the "New Query" button a primary button. Tests related to the removed generate CRUD page feature have been removed.

Fixes #31801

Automation

/ok-to-test tags="@tag.Datasource"

:mag: Cypress test results

[!TIP] 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉 Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/9644141773 Commit: b1671022d7eee6a676103281dc2e7f5df8bfd513 Cypress dashboard. Tags: @tag.Datasource

Communication

Should the DevRel and Marketing teams inform users about this change?

  • [ ] Yes
  • [ ] No

Summary by CodeRabbit

  • Bug Fixes

    • Removed outdated test cases that were no longer relevant or causing issues in MongoDB and S3 CRUD operations.
  • New Tests

    • Added HideGeneratePageButton.test.tsx to verify the visibility of buttons based on feature flags and user permissions.
  • Refactor

    • Improved code structure by reordering imports and simplifying logic in various datasource-related components.
    • Removed deprecated methods and functions related to page generation from datasource components.

jacquesikot avatar Jun 19 '24 13:06 jacquesikot

Here is a more advanced example that shows some issues when using globmatching where the glob can be expanded accidentally:

Here's a snippet or screenshot that shows the problem:

#!/bin/busybox sh

testglob() {
    local var1="barfoobar"
    local var2="foobar"
    local var3="barfoo"
    local var4="bar"

    [[ ${var1} == *foo* ]] && echo "(unquoted pattern) var1 true" || echo "(unquoted pattern) var1 false"
    [[ ${var2} == *foo* ]] && echo "(unquoted pattern) var2 true" || echo "(unquoted pattern) var2 false"
    [[ ${var3} == *foo* ]] && echo "(unquoted pattern) var3 true" || echo "(unquoted pattern) var3 false"
    [[ ${var4} == *foo* ]] && echo "(unquoted pattern) var4 true" || echo "(unquoted pattern) var4 false"

    [[ ${var1} == "*foo*" ]] && echo "(quoted pattern) var1 true" || echo "(quoted pattern) var1 false"
    [[ ${var2} == "*foo*" ]] && echo "(quoted pattern) var2 true" || echo "(quoted pattern) var2 false"
    [[ ${var3} == "*foo*" ]] && echo "(quoted pattern) var3 true" || echo "(quoted pattern) var3 false"
    [[ ${var4} == "*foo*" ]] && echo "(quoted pattern) var4 true" || echo "(quoted pattern) var4 false"
}

rm -f barfoobar xfoox && echo "Neither file barfoobar nor xfoox exist"
testglob
echo

touch barfoobar && echo "File barfoobar exists"
testglob
echo

touch xfoox && echo "File barfoobar and file xfoox exists"
testglob
echo

That will output:

Neither file barfoobar nor xfoox exist
(unquoted pattern) var1 true
(unquoted pattern) var2 true
(unquoted pattern) var3 true
(unquoted pattern) var4 false
(quoted pattern) var1 true
(quoted pattern) var2 true
(quoted pattern) var3 true
(quoted pattern) var4 false

File barfoobar exists
(unquoted pattern) var1 true
(unquoted pattern) var2 false
(unquoted pattern) var3 false
(unquoted pattern) var4 false
(quoted pattern) var1 true
(quoted pattern) var2 true
(quoted pattern) var3 true
(quoted pattern) var4 false

File barfoobar and file xfoox exists
sh: xfoox: unknown operand
(unquoted pattern) var1 false
sh: xfoox: unknown operand
(unquoted pattern) var2 false
sh: xfoox: unknown operand
(unquoted pattern) var3 false
sh: xfoox: unknown operand
(unquoted pattern) var4 false
(quoted pattern) var1 true
(quoted pattern) var2 true
(quoted pattern) var3 true
(quoted pattern) var4 false

Here's what shellcheck currently says:

$ shellcheck myscript
 
[Line 10:](javascript:setPosition(10, 19))
    [[ ${var1} == *foo* ]] && echo "(unquoted pattern) var1 true" || echo "(unquoted pattern) var1 false"
                  ^-- [SC2330](https://www.shellcheck.net/wiki/SC2330) (error): BusyBox [[ .. ]] does not support glob matching. Use a case statement.
 
[Line 11:](javascript:setPosition(11, 19))
    [[ ${var2} == *foo* ]] && echo "(unquoted pattern) var2 true" || echo "(unquoted pattern) var2 false"
                  ^-- [SC2330](https://www.shellcheck.net/wiki/SC2330) (error): BusyBox [[ .. ]] does not support glob matching. Use a case statement.
 
[Line 12:](javascript:setPosition(12, 19))
    [[ ${var3} == *foo* ]] && echo "(unquoted pattern) var3 true" || echo "(unquoted pattern) var3 false"
                  ^-- [SC2330](https://www.shellcheck.net/wiki/SC2330) (error): BusyBox [[ .. ]] does not support glob matching. Use a case statement.
 
[Line 13:](javascript:setPosition(13, 19))
    [[ ${var4} == *foo* ]] && echo "(unquoted pattern) var4 true" || echo "(unquoted pattern) var4 false"
                  ^-- [SC2330](https://www.shellcheck.net/wiki/SC2330) (error): BusyBox [[ .. ]] does not support glob matching. Use a case statement.

Here's what I wanted or expected to see:

In the above example, the error seems valid, as it requires quoting to work properly. But it should very likely be reworded.

grische avatar Jun 10 '24 08:06 grische