-
-
Notifications
You must be signed in to change notification settings - Fork 719
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
Add "Affiliate Sales Data" connected app option #12676
Changes from all commits
9a4051f
85d5e2e
bbe22bb
9b37eac
9d89b47
5d0f55b
27e53f9
d3c5e23
1742d28
da7bbcf
e9d7a0b
df81e8e
c5fc621
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,7 +5,31 @@ | |
# Here we store keys and links to access the app. | ||
class ConnectedApp < ApplicationRecord | ||
belongs_to :enterprise | ||
after_destroy :disconnect | ||
|
||
scope :discover_regen, -> { where(type: "ConnectedApp") } | ||
scope :affiliate_sales_data, -> { where(type: "ConnectedApps::AffiliateSalesData") } | ||
|
||
scope :connecting, -> { where(data: nil) } | ||
scope :ready, -> { where.not(data: nil) } | ||
|
||
def connecting? | ||
data.nil? | ||
end | ||
|
||
def ready? | ||
!connecting? | ||
end | ||
|
||
def connect(api_key:, channel:) | ||
ConnectAppJob.perform_later(self, api_key, channel:) | ||
end | ||
|
||
def disconnect | ||
WebhookDeliveryJob.perform_later( | ||
data["destroy"], | ||
"disconnect-app", | ||
nil | ||
) | ||
end | ||
Comment on lines
+24
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah it feels wrong sending the websocket channel to the model. But with a service, I guess the controller has to know about the subtypes, and make the decision what to do with them, which doesn't seem any better. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean you could have a service that knows how to connect/disconnect each type of connected app, but that would add some indirection. |
||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
# frozen_string_literal: true | ||
|
||
# An enterprise can opt-in for their data to be included in the affiliate_sales_data endpoint | ||
# | ||
module ConnectedApps | ||
class AffiliateSalesData < ConnectedApp | ||
def connect(_opts) | ||
update! data: true # not-nil value indicates it is ready | ||
end | ||
Comment on lines
+7
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we set this value in the constructor so that we don't have to update the record again? Doesn't really matter though. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good thinking |
||
|
||
def disconnect; end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,30 +1,4 @@ | ||
%div{ id: "connected-app-discover-regen", class: "enterprise_#{enterprise.id}" } | ||
.connected-app__head | ||
%div | ||
%h3= t ".title" | ||
%p= t ".tagline" | ||
%div | ||
- if enterprise.connected_apps.empty? | ||
= button_to t(".enable"), admin_enterprise_connected_apps_path(enterprise.id), method: :post, disabled: !managed_by_user?(enterprise) | ||
-# This is only seen by super-admins: | ||
%em= t(".need_to_be_manager") unless managed_by_user?(enterprise) | ||
- elsif enterprise.connected_apps.connecting.present? | ||
%button{ disabled: true } | ||
%i.spinner.fa.fa-spin.fa-circle-o-notch | ||
| ||
= t ".loading" | ||
- else | ||
= button_to t(".disable"), admin_enterprise_connected_app_path(0, enterprise_id: enterprise.id), method: :delete | ||
|
||
.connected-app__connection | ||
- if enterprise.connected_apps.ready.present? | ||
.connected-app__note | ||
- link = enterprise.connected_apps[0].data["link"] | ||
%p= t ".note" | ||
%div | ||
%a{ href: link, target: "_blank", class: "button secondary" } | ||
= t ".link_label" | ||
|
||
%hr | ||
.connected-app__description | ||
= t ".description_html" | ||
= render partial: "/admin/enterprises/form/connected_apps/discover_regen", | ||
locals: { enterprise:, connected_app: enterprise.connected_apps.discover_regen.first } | ||
= render partial: "/admin/enterprises/form/connected_apps/affiliate_sales_data", | ||
locals: { enterprise:, connected_app: enterprise.connected_apps.affiliate_sales_data.first } |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
%section.connected_app | ||
.connected-app__head | ||
%div | ||
%h3= t ".title" | ||
%p= t ".tagline" | ||
%div | ||
- if connected_app.nil? | ||
= button_to t(".enable"), admin_enterprise_connected_apps_path(enterprise.id, type: "ConnectedApps::AffiliateSalesData"), method: :post, disabled: !managed_by_user?(enterprise) | ||
-# This is only seen by super-admins: | ||
%em= t(".need_to_be_manager") unless managed_by_user?(enterprise) | ||
- else | ||
= button_to t(".disable"), admin_enterprise_connected_app_path(connected_app.id, enterprise_id: enterprise.id), method: :delete | ||
%hr | ||
.connected-app__description | ||
= t ".description_html" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
%section.connected_app{ id: "connected-app-discover-regen", class: "enterprise_#{enterprise.id}" } | ||
.connected-app__head | ||
%div | ||
%h3= t ".title" | ||
%p= t ".tagline" | ||
%div | ||
- if connected_app.nil? | ||
= button_to t(".enable"), admin_enterprise_connected_apps_path(enterprise.id), method: :post, disabled: !managed_by_user?(enterprise) | ||
-# This is only seen by super-admins: | ||
%em= t(".need_to_be_manager") unless managed_by_user?(enterprise) | ||
- elsif connected_app&.connecting? | ||
%button{ disabled: true } | ||
%i.spinner.fa.fa-spin.fa-circle-o-notch | ||
| ||
= t ".loading" | ||
- else | ||
= button_to t(".disable"), admin_enterprise_connected_app_path(connected_app.id, enterprise_id: enterprise.id), method: :delete | ||
|
||
.connected-app__connection | ||
- if connected_app&.ready? | ||
.connected-app__note | ||
- link = connected_app.data["link"] | ||
%p= t ".note" | ||
%div | ||
%a{ href: link, target: "_blank", class: "button secondary" } | ||
= t ".link_label" | ||
|
||
%hr | ||
.connected-app__description | ||
= t ".description_html" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
class AddTypeToConnectedApps < ActiveRecord::Migration[7.0] | ||
def change | ||
add_column :connected_apps, :type, :string, default: "ConnectedApp", null: false | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,18 @@ namespace :ofn do | |
enterprise.destroy | ||
end | ||
|
||
namespace :enterprises do | ||
desc "Activate connected app type for ALL enterprises" | ||
task :activate_connected_app_type, [:type] => :environment do |_task, args| | ||
Enterprise.find_each do |enterprise| | ||
next if enterprise.connected_apps.public_send(args.type.underscore).exists? | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am wondering if we should whitelist the type param here ? Probably not necessary as it will blow up if you are trying to use a scope that doesn't exist. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I figured for a rake task that requires system access to execute, we don't need to add much protection. |
||
|
||
"ConnectedApps::#{args.type.camelize}".constantize.new(enterprise:).connect({}) | ||
puts "Enterprise #{enterprise.id} connected." | ||
end | ||
end | ||
end | ||
|
||
namespace :dev do | ||
desc 'export enterprises to CSV' | ||
task export_enterprises: :environment do | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess that the more generic alternative would be
ConnectedApp.of(enterprise).first
. We'll see with more usage what will be better.