Merge pull request #11670 from metabase/fix-11343

Fix segment editing

frontend/src/metabase/admin/datamodel/containers/MetricApp.jsx

@@ -15,6 +15,7 @@ import MetricForm from "./MetricForm";
 const mapDispatchToProps = {
   updatePreviewSummary,
   createMetric: Metrics.actions.create,
+  updateMetric: Metrics.actions.update,
   onChangeLocation: push,
 };
 
@@ -23,15 +24,12 @@ const mapStateToProps = (state, props) => ({
   previewSummary: getPreviewSummary(state),
 });
 
-@Metrics.load({
-  id: (state, props) => parseInt(props.params.id),
-  wrapped: true,
-})
+@Metrics.load({ id: (state, props) => parseInt(props.params.id) })
 @Tables.load({ id: (state, props) => props.metric.table_id, wrapped: true })
 @withTableMetadataLoaded
 class UpdateMetricForm extends Component {
   onSubmit = async metric => {
-    await this.props.metric.update(metric);
+    await this.props.updateMetric(metric);
     MetabaseAnalytics.trackEvent("Data Model", "Metric Updated");
     const { id: tableId, db_id: databaseId } = this.props.table;
     this.props.onChangeLocation(
@@ -40,7 +38,14 @@ class UpdateMetricForm extends Component {
   };
 
   render() {
-    return <MetricForm {...this.props} onSubmit={this.onSubmit} />;
+    const { metric, ...props } = this.props;
+    return (
+      <MetricForm
+        {...props}
+        metric={metric.getPlainObject()}
+        onSubmit={this.onSubmit}
+      />
+    );
   }
 }
 

frontend/src/metabase/admin/datamodel/containers/SegmentApp.jsx

@@ -16,6 +16,7 @@ import Tables from "metabase/entities/tables";
 const mapDispatchToProps = {
   updatePreviewSummary,
   createSegment: Segments.actions.create,
+  updateSegment: Segments.actions.update,
   onChangeLocation: push,
 };
 
@@ -24,15 +25,12 @@ const mapStateToProps = (state, props) => ({
   previewSummary: getPreviewSummary(state),
 });
 
-@Segments.load({
-  id: (state, props) => parseInt(props.params.id),
-  wrapped: true,
-})
+@Segments.load({ id: (state, props) => parseInt(props.params.id) })
 @Tables.load({ id: (state, props) => props.segment.table_id, wrapped: true })
 @withTableMetadataLoaded
 class UpdateSegmentForm extends Component {
   onSubmit = async segment => {
-    await this.props.segment.update(segment);
+    await this.props.updateSegment(segment);
     MetabaseAnalytics.trackEvent("Data Model", "Segment Updated");
     const { id: tableId, db_id: databaseId } = this.props.table;
     this.props.onChangeLocation(
@@ -41,7 +39,14 @@ class UpdateSegmentForm extends Component {
   };
 
   render() {
-    return <SegmentForm {...this.props} onSubmit={this.onSubmit} />;
+    const { segment, ...props } = this.props;
+    return (
+      <SegmentForm
+        {...props}
+        segment={segment.getPlainObject()}
+        onSubmit={this.onSubmit}
+      />
+    );
   }
 }
 

frontend/test/metabase/scenarios/datamodel/segments.cy.spec.js

+import { signInAsAdmin } from "__support__/cypress";
+
+describe("Segments", () => {
+  beforeEach(() => {
+    signInAsAdmin();
+    cy.viewport(1400, 860);
+  });
+
+  it("should create a segment", () => {
+    cy.visit("/admin");
+    cy.contains("Data Model").click();
+    cy.contains("Orders").click();
+
+    // empty state displays message
+    cy.contains(
+      "Create segments to add them to the Filter dropdown in the query builder",
+    );
+
+    // redirected to segment form
+    cy.contains("Add a Segment").click();
+    cy.url().should("match", /segment\/create\?table=2$/);
+    cy.contains("Create Your Segment");
+
+    // filter to orders with total under 100
+    cy.contains("Add filters").click();
+    cy.contains("Total").click();
+    cy.contains("Equal to").click();
+    cy.contains("Less than").click();
+    cy.get('[placeholder="Enter a number"]').type("100");
+    cy.get(".PopoverBody")
+      .contains("Add filter")
+      .click();
+
+    //
+    cy.contains("12765 rows");
+
+    // fill in name/description
+    cy.get('[name="name"]').type("orders <100");
+    cy.get('[name="description"]').type("All orders with a total under $100.");
+
+    // saving bounces you back and you see new segment in the list
+    cy.contains("Save changes").click();
+    cy.url().should("match", /datamodel\/database\/1\/table\/2$/);
+    cy.contains("orders <100");
+    cy.contains("Filtered by Total");
+  });
+
+  it("should update that segment", () => {
+    // visit table's data model page and click to edit the segment
+    cy.visit("/admin/datamodel/database/1/table/2");
+    cy.contains("orders <100")
+      .parent()
+      .find(".Icon-ellipsis")
+      .click();
+    cy.contains("Edit Segment").click();
+
+    // update the filter from "< 100" to "> 10"
+    cy.url().should("match", /segment\/1$/);
+    cy.contains("Edit Your Segment");
+    cy.contains(/Total\s+is less than/).click();
+    cy.get(".PopoverBody")
+      .contains("Less than")
+      .click();
+    cy.get(".PopoverBody")
+      .contains("Greater than")
+      .click();
+    cy.get(".PopoverBody input").type("{SelectAll}10");
+    cy.get(".PopoverBody")
+      .contains("Update filter")
+      .click();
+
+    // confirm that the preview updated
+    cy.contains("18703 rows");
+
+    // update name and description, set a revision note, and save the update
+    cy.get('[name="name"]')
+      .clear()
+      .type("orders >10");
+    cy.get('[name="description"]')
+      .clear()
+      .type("All orders with a total over $10.");
+    cy.get('[name="revision_message"]').type("time for a change");
+    cy.contains("Save changes").click();
+
+    // get redirected to previous page and see the new segment name
+    cy.url().should("match", /datamodel\/database\/1\/table\/2$/);
+    cy.contains("orders >10");
+
+    // clean up
+    cy.contains("orders >10")
+      .parent()
+      .find(".Icon-ellipsis")
+      .click();
+    cy.contains("Retire Segment").click();
+    cy.get(".ModalBody textarea").type("delete it");
+    cy.get(".ModalBody")
+      .contains("button", "Retire")
+      .click();
+  });
+});