Implement Mark as Deleted for Backend Api #1144

duduribeiro · 2019-09-02T21:11:13Z

What this PR does / why we need it:
This commit changed the behave of the destroy action from the
Backend API controller to make it delete the object in background.

Which issue(s) this PR fixes

THREESCALE-3343

hallelujah · 2019-09-03T09:24:46Z

So I am thinking of not really use any other column but only do it when the service is not a product

hallelujah · 2019-09-03T09:27:58Z

I do not think this column is needed at all,
If the only service tied to it is an old service

app/controllers/provider/admin/backend_apis_controller.rb

hallelujah · 2019-09-04T15:07:44Z

So decision is to use the mark as deleted because of the points that @Martouta issued.
Those are really valid, and we should just follow what we have in Service

codecov · 2019-09-04T15:38:29Z

Codecov Report

❗ No coverage uploaded for pull request base (master@4c3d753). Click here to learn what that means.
The diff coverage is 52.63%.

@@            Coverage Diff            @@
##             master    #1144   +/-   ##
=========================================
  Coverage          ?   67.65%           
=========================================
  Files             ?     1590           
  Lines             ?    43312           
  Branches          ?        0           
=========================================
  Hits              ?    29301           
  Misses            ?    14011           
  Partials          ?        0

Impacted Files	Coverage Δ
...ers/provider/admin/backend_apis/base_controller.rb	`85.71% <0%> (ø)`
...trollers/provider/admin/backend_apis_controller.rb	`47.5% <0%> (ø)`
...p/controllers/admin/api/backend_apis_controller.rb	`61.29% <0%> (ø)`
...llers/admin/api/backend_apis/metrics_controller.rb	`60% <0%> (ø)`
app/models/backend_api.rb	`91.66% <90.9%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c3d753...315ca82. Read the comment docs.

Martouta

Awesome 🥇 🚀

test/integration/provider/admin/backend_apis_controller_test.rb

hallelujah · 2019-09-21T13:14:59Z

db/migrate/20190904144157_add_state_to_backend_apis.rb

+  disable_ddl_transaction! if System::Database.postgres?
+
+  def change
+    add_column :backend_apis, :state, :string


We could add a default here I think. On SaaS:

irb(main):001:0> BackendApi.count => 37183

It will be blocking but reasonably, and in fact it might not block at all, there are very few requests that modifies the private_endpoint

hallelujah · 2019-09-21T13:15:21Z

db/migrate/20190904144307_backfill_state_for_backend_apis.rb

+class BackfillStateForBackendApis < ActiveRecord::Migration
+  disable_ddl_transaction!
+
+  def change


Please extract it to a rake task so we can do it after migration from 2.6 to 2.7

hallelujah · 2019-09-21T13:24:07Z

app/models/backend_api.rb

@@ -16,6 +17,7 @@ class BackendApi < ApplicationRecord
  delegate :default_api_backend, to: :class

  validates :name,        length: { maximum: 511 }, presence: true


So I was wondering

Why do we have 511 characters here????

hahahahaha good question 😂 i will check 😄

porta/db/schema.rb

Line 235 in 7131eb1

t.string "name", limit: 511, null: false

c2204dc#diff-e2de550cd7105b0cc383c49f401ca88dR6

THREESCALE-3343 This commit changed the behave of the destroy action from the Backend API controller to make it delete the object in background.

Martouta · 2019-09-22T14:39:17Z

db/migrate/20190904144157_add_state_to_backend_apis.rb

+  disable_ddl_transaction! if System::Database.postgres?
+
+  def change
+    safety_assured { add_column :backend_apis, :state, :string, default: :published, null: false }


This will fill up the DB 😉

duduribeiro self-assigned this Sep 2, 2019

duduribeiro added the APIAP label Sep 2, 2019

duduribeiro force-pushed the implement_mark_as_deleted_for_backend_apis branch from ed36f41 to 9a59922 Compare September 2, 2019 21:38

duduribeiro force-pushed the implement_mark_as_deleted_for_backend_apis branch from 9a59922 to b3cb4cf Compare September 3, 2019 18:14

duduribeiro marked this pull request as ready for review September 3, 2019 18:19

duduribeiro requested a review from a team September 3, 2019 18:19

Martouta reviewed Sep 4, 2019

View reviewed changes

app/controllers/provider/admin/backend_apis_controller.rb Outdated Show resolved Hide resolved

hallelujah mentioned this pull request Sep 4, 2019

[API Endpoints] Backend API CRUD #1065

Merged

Martouta added the Priority: Blocker This PR is a blocker. Everyone should review and help to close it ASAP. label Sep 4, 2019

duduribeiro force-pushed the implement_mark_as_deleted_for_backend_apis branch from b3cb4cf to 2173fdc Compare September 4, 2019 15:38

duduribeiro force-pushed the implement_mark_as_deleted_for_backend_apis branch from 2173fdc to ec5d8f1 Compare September 4, 2019 17:34

Martouta self-requested a review September 5, 2019 08:12

Martouta previously approved these changes Sep 5, 2019

View reviewed changes

Martouta mentioned this pull request Sep 5, 2019

[DO NOT MERGE] Destroy backend apis belonging to the service when it is deleted #1127

Closed

hallelujah changed the title ~~Implement Mark as Deleted for Backend Api~~ [DO NOT MERGE]Implement Mark as Deleted for Backend Api Sep 5, 2019

Martouta requested a review from a team September 5, 2019 08:21

Martouta added the on hold Waiting for or blocked by something else. label Sep 5, 2019

duduribeiro dismissed Martouta’s stale review via 7c0f080 September 5, 2019 13:28

duduribeiro force-pushed the implement_mark_as_deleted_for_backend_apis branch from ec5d8f1 to 7c0f080 Compare September 5, 2019 13:28

Martouta reviewed Sep 5, 2019

View reviewed changes

test/integration/provider/admin/backend_apis_controller_test.rb Outdated Show resolved Hide resolved

duduribeiro force-pushed the implement_mark_as_deleted_for_backend_apis branch from 7c0f080 to 0b0eb50 Compare September 5, 2019 19:29

Martouta previously approved these changes Sep 5, 2019

View reviewed changes

duduribeiro mentioned this pull request Sep 10, 2019

Make independent mapping rules always enabled when APIAP is on #1133

Merged

duduribeiro dismissed Martouta’s stale review via 9fd4e84 September 12, 2019 14:57

duduribeiro force-pushed the implement_mark_as_deleted_for_backend_apis branch from 0b0eb50 to 9fd4e84 Compare September 12, 2019 14:57

Martouta mentioned this pull request Sep 12, 2019

[API] Backend API with deletion in background through state #1178

Merged

Martouta added the Priority: Blocker This PR is a blocker. Everyone should review and help to close it ASAP. label Sep 20, 2019

Martouta force-pushed the implement_mark_as_deleted_for_backend_apis branch 2 times, most recently from 315ca82 to 87b4dec Compare September 21, 2019 11:39

hallelujah reviewed Sep 21, 2019

View reviewed changes

Martouta changed the title ~~Implement Mark as Deleted for Backend Api~~ [WIP] Implement Mark as Deleted for Backend Api Sep 21, 2019

Martouta force-pushed the implement_mark_as_deleted_for_backend_apis branch 2 times, most recently from 805b0e7 to 35af93a Compare September 21, 2019 14:52

Martouta changed the title ~~[WIP] Implement Mark as Deleted for Backend Api~~ Implement Mark as Deleted for Backend Api Sep 21, 2019

Martouta requested review from a team and hallelujah September 21, 2019 15:28

duduribeiro and others added 5 commits September 21, 2019 18:01

Implement Mark as Deleted for Backend API

981652d

THREESCALE-3343 This commit changed the behave of the destroy action from the Backend API controller to make it delete the object in background.

[Fix] UI BackendAPi Index does not exist anymore since #1130 (#1181)

5e880d0

[API] Backend API with deletion in background through state

f83034e

Fix Test to order BackendAPI before fetching the 2nd

5689287

Improve test: #accessible does not return deleted backend apis

5a6037a

Martouta force-pushed the implement_mark_as_deleted_for_backend_apis branch from e4f1b30 to cbe0c84 Compare September 21, 2019 16:01

Martouta and others added 4 commits September 21, 2019 18:35

Controllers under BackendAPI only find it among the accessible ones

7f0bac3

Original schemas

3b331f2

Migration with default

8a31e87

[schema] Add oracle schema

c98f764

Martouta force-pushed the implement_mark_as_deleted_for_backend_apis branch from cbe0c84 to c98f764 Compare September 21, 2019 16:35

Martouta added Priority: High Closes a JIRA issue in our sprint and removed Priority: Blocker This PR is a blocker. Everyone should review and help to close it ASAP. labels Sep 22, 2019

hallelujah approved these changes Sep 22, 2019

View reviewed changes

Martouta reviewed Sep 22, 2019

View reviewed changes

Martouta merged commit f02c25a into master Sep 22, 2019

Martouta deleted the implement_mark_as_deleted_for_backend_apis branch September 22, 2019 14:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement Mark as Deleted for Backend Api #1144

Implement Mark as Deleted for Backend Api #1144

duduribeiro commented Sep 2, 2019 •

edited by Martouta

hallelujah commented Sep 3, 2019

hallelujah commented Sep 3, 2019

hallelujah commented Sep 4, 2019

codecov bot commented Sep 4, 2019 •

edited

Martouta left a comment

hallelujah Sep 21, 2019

hallelujah Sep 21, 2019 •

edited

hallelujah Sep 21, 2019

Martouta Sep 21, 2019

Martouta Sep 21, 2019

Martouta Sep 22, 2019

		@@ -16,6 +17,7 @@ class BackendApi < ApplicationRecord
		delegate :default_api_backend, to: :class

		validates :name, length: { maximum: 511 }, presence: true

Implement Mark as Deleted for Backend Api #1144

Implement Mark as Deleted for Backend Api #1144

Conversation

duduribeiro commented Sep 2, 2019 • edited by Martouta

hallelujah commented Sep 3, 2019

hallelujah commented Sep 3, 2019

hallelujah commented Sep 4, 2019

codecov bot commented Sep 4, 2019 • edited

Codecov Report

Martouta left a comment

Choose a reason for hiding this comment

hallelujah Sep 21, 2019

Choose a reason for hiding this comment

hallelujah Sep 21, 2019 • edited

Choose a reason for hiding this comment

hallelujah Sep 21, 2019

Choose a reason for hiding this comment

Martouta Sep 21, 2019

Choose a reason for hiding this comment

Martouta Sep 21, 2019

Choose a reason for hiding this comment

Martouta Sep 22, 2019

Choose a reason for hiding this comment

duduribeiro commented Sep 2, 2019 •

edited by Martouta

codecov bot commented Sep 4, 2019 •

edited

hallelujah Sep 21, 2019 •

edited