fix(app-store): address follow-up review comments
This commit is contained in:
@@ -2,12 +2,15 @@ class Inboxes::FetchAppStoreReviewInboxesJob < ApplicationJob
|
|||||||
queue_as :scheduled_jobs
|
queue_as :scheduled_jobs
|
||||||
|
|
||||||
def perform
|
def perform
|
||||||
Inbox.where(channel_type: 'Channel::AppStore').find_each(batch_size: 100) do |inbox|
|
Inbox.includes(:account, :channel).where(channel_type: 'Channel::AppStore').find_each(batch_size: 100) do |inbox|
|
||||||
next if inbox.account.suspended?
|
account = inbox.account
|
||||||
next unless inbox.account.feature_enabled?(:channel_app_store)
|
channel = inbox.channel
|
||||||
next unless inbox.channel.sync_due?
|
|
||||||
|
|
||||||
::Inboxes::FetchAppStoreReviewsJob.perform_later(inbox.channel)
|
next if account.suspended?
|
||||||
|
next unless account.feature_enabled?(:channel_app_store)
|
||||||
|
next unless channel.sync_due?
|
||||||
|
|
||||||
|
::Inboxes::FetchAppStoreReviewsJob.perform_later(channel)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -32,15 +32,11 @@ class Channel::AppStore < ApplicationRecord
|
|||||||
end
|
end
|
||||||
|
|
||||||
def fetch_reviews
|
def fetch_reviews
|
||||||
app_store_client.fetch_reviews
|
app_store_client.fetch_reviews(since: last_synced_at)
|
||||||
end
|
end
|
||||||
|
|
||||||
def reply_to_review(review_id, response_body, response_id: nil)
|
def reply_to_review(review_id, response_body)
|
||||||
response = if response_id.present?
|
response = app_store_client.create_or_update_review_response(review_id, response_body)
|
||||||
app_store_client.update_review_response(review_id, response_body)
|
|
||||||
else
|
|
||||||
app_store_client.create_review_response(review_id, response_body)
|
|
||||||
end
|
|
||||||
|
|
||||||
response['id']
|
response['id']
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -8,7 +8,7 @@ class AppStore::SendOnAppStoreService < Base::SendOnChannelService
|
|||||||
def perform_reply
|
def perform_reply
|
||||||
validate_feature_enabled!
|
validate_feature_enabled!
|
||||||
validate_message_support!
|
validate_message_support!
|
||||||
source_id = channel.reply_to_review(review_id, reply_content, response_id: existing_response_id)
|
source_id = channel.reply_to_review(review_id, reply_content)
|
||||||
message.update!(source_id: source_id) if source_id.present?
|
message.update!(source_id: source_id) if source_id.present?
|
||||||
Messages::StatusUpdateService.new(message, 'delivered').perform
|
Messages::StatusUpdateService.new(message, 'delivered').perform
|
||||||
rescue StandardError => e
|
rescue StandardError => e
|
||||||
@@ -33,13 +33,4 @@ class AppStore::SendOnAppStoreService < Base::SendOnChannelService
|
|||||||
def reply_content
|
def reply_content
|
||||||
message.outgoing_content.presence || message.content
|
message.outgoing_content.presence || message.content
|
||||||
end
|
end
|
||||||
|
|
||||||
def existing_response_id
|
|
||||||
message.conversation.messages
|
|
||||||
.outgoing
|
|
||||||
.where.not(id: message.id)
|
|
||||||
.where.not(source_id: [nil, ''])
|
|
||||||
.order(created_at: :desc)
|
|
||||||
.pick(:source_id)
|
|
||||||
end
|
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -7,14 +7,18 @@ class AppStoreConnect::Client
|
|||||||
get("/v1/apps/#{channel.app_id}")['data']
|
get("/v1/apps/#{channel.app_id}")['data']
|
||||||
end
|
end
|
||||||
|
|
||||||
def fetch_reviews
|
def fetch_reviews(since: nil)
|
||||||
reviews = []
|
reviews = []
|
||||||
next_url = nil
|
next_url = nil
|
||||||
|
|
||||||
loop do
|
loop do
|
||||||
payload = next_url ? get_url(next_url) : get(reviews_path, reviews_query)
|
payload = next_url ? get_url(next_url) : get(reviews_path, reviews_query)
|
||||||
included = Array(payload['included'])
|
included = Array(payload['included'])
|
||||||
reviews.concat(Array(payload['data']).map { |review| normalize_review(review, included) })
|
review_payloads = Array(payload['data']).map { |review| normalize_review(review, included) }
|
||||||
|
fresh_payloads = fresh_review_payloads(review_payloads, since)
|
||||||
|
reviews.concat(fresh_payloads)
|
||||||
|
|
||||||
|
break if since.present? && fresh_payloads.size < review_payloads.size
|
||||||
|
|
||||||
next_url = payload.dig('links', 'next')
|
next_url = payload.dig('links', 'next')
|
||||||
break if next_url.blank?
|
break if next_url.blank?
|
||||||
@@ -24,10 +28,10 @@ class AppStoreConnect::Client
|
|||||||
end
|
end
|
||||||
|
|
||||||
def create_review_response(review_id, response_body)
|
def create_review_response(review_id, response_body)
|
||||||
post('/v1/customerReviewResponses', review_response_payload(review_id, response_body))['data']
|
create_or_update_review_response(review_id, response_body)
|
||||||
end
|
end
|
||||||
|
|
||||||
def update_review_response(review_id, response_body)
|
def create_or_update_review_response(review_id, response_body)
|
||||||
post('/v1/customerReviewResponses', review_response_payload(review_id, response_body))['data']
|
post('/v1/customerReviewResponses', review_response_payload(review_id, response_body))['data']
|
||||||
end
|
end
|
||||||
|
|
||||||
@@ -55,6 +59,21 @@ class AppStoreConnect::Client
|
|||||||
}
|
}
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def fresh_review_payloads(review_payloads, since)
|
||||||
|
return review_payloads if since.blank?
|
||||||
|
|
||||||
|
review_payloads.select { |review_payload| review_created_after?(review_payload, since) }
|
||||||
|
end
|
||||||
|
|
||||||
|
def review_created_after?(review_payload, since)
|
||||||
|
created_at = Time.zone.parse(review_payload.dig('review', 'attributes', 'createdDate').to_s)
|
||||||
|
return true if created_at.blank?
|
||||||
|
|
||||||
|
created_at > since
|
||||||
|
rescue StandardError
|
||||||
|
true
|
||||||
|
end
|
||||||
|
|
||||||
def get(path, query = {})
|
def get(path, query = {})
|
||||||
request(:get, "#{Channel::AppStore::API_BASE_URL}#{path}", query: query)
|
request(:get, "#{Channel::AppStore::API_BASE_URL}#{path}", query: query)
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -15,6 +15,5 @@ class RepurposeMessageReplyToFlagForChannelAppStore < ActiveRecord::Migration[7.
|
|||||||
|
|
||||||
config.value = config.value.reject { |feature| feature['name'] == 'message_reply_to' }
|
config.value = config.value.reject { |feature| feature['name'] == 'message_reply_to' }
|
||||||
config.save!
|
config.save!
|
||||||
GlobalConfig.clear_cache
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
@@ -26,7 +26,7 @@ RSpec.describe AppStore::SendOnAppStoreService do
|
|||||||
|
|
||||||
described_class.new(message: message).perform
|
described_class.new(message: message).perform
|
||||||
|
|
||||||
expect(channel).to have_received(:reply_to_review).with('review-1', 'Thanks', response_id: nil)
|
expect(channel).to have_received(:reply_to_review).with('review-1', 'Thanks')
|
||||||
expect(message.reload.source_id).to eq('response-1')
|
expect(message.reload.source_id).to eq('response-1')
|
||||||
expect(Messages::StatusUpdateService).to have_received(:new).with(message, 'delivered')
|
expect(Messages::StatusUpdateService).to have_received(:new).with(message, 'delivered')
|
||||||
end
|
end
|
||||||
@@ -40,7 +40,7 @@ RSpec.describe AppStore::SendOnAppStoreService do
|
|||||||
|
|
||||||
described_class.new(message: message).perform
|
described_class.new(message: message).perform
|
||||||
|
|
||||||
expect(channel).to have_received(:reply_to_review).with('review-1', 'Updated reply', response_id: 'response-1')
|
expect(channel).to have_received(:reply_to_review).with('review-1', 'Updated reply')
|
||||||
expect(Messages::StatusUpdateService).to have_received(:new).with(message, 'delivered')
|
expect(Messages::StatusUpdateService).to have_received(:new).with(message, 'delivered')
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|||||||
@@ -43,6 +43,9 @@ RSpec.describe AppStoreConnect::Client do
|
|||||||
{
|
{
|
||||||
id: 'review-1',
|
id: 'review-1',
|
||||||
type: 'customerReviews',
|
type: 'customerReviews',
|
||||||
|
attributes: {
|
||||||
|
createdDate: '2026-05-20T10:00:00-00:00'
|
||||||
|
},
|
||||||
relationships: {
|
relationships: {
|
||||||
response: {
|
response: {
|
||||||
data: {
|
data: {
|
||||||
@@ -72,6 +75,41 @@ RSpec.describe AppStoreConnect::Client do
|
|||||||
expect(review_payload['response']['id']).to eq('response-1')
|
expect(review_payload['response']['id']).to eq('response-1')
|
||||||
end
|
end
|
||||||
|
|
||||||
|
it 'stops fetching when a scheduled sync reaches already synced reviews' do
|
||||||
|
stub_request(:get, 'https://api.appstoreconnect.apple.com/v1/apps/123456789/customerReviews')
|
||||||
|
.with(query: { include: 'response', limit: '200', sort: '-createdDate' })
|
||||||
|
.to_return(
|
||||||
|
status: 200,
|
||||||
|
body: {
|
||||||
|
data: [
|
||||||
|
{
|
||||||
|
id: 'review-1',
|
||||||
|
type: 'customerReviews',
|
||||||
|
attributes: {
|
||||||
|
createdDate: '2026-05-20T10:00:00-00:00'
|
||||||
|
}
|
||||||
|
},
|
||||||
|
{
|
||||||
|
id: 'review-2',
|
||||||
|
type: 'customerReviews',
|
||||||
|
attributes: {
|
||||||
|
createdDate: '2026-05-19T10:00:00-00:00'
|
||||||
|
}
|
||||||
|
}
|
||||||
|
],
|
||||||
|
links: {
|
||||||
|
next: 'https://api.appstoreconnect.apple.com/v1/apps/123456789/customerReviews?page=2'
|
||||||
|
}
|
||||||
|
}.to_json,
|
||||||
|
headers: { 'Content-Type' => 'application/json' }
|
||||||
|
)
|
||||||
|
|
||||||
|
review_payloads = described_class.new(channel: channel).fetch_reviews(since: Time.zone.parse('2026-05-20T00:00:00-00:00'))
|
||||||
|
|
||||||
|
expect(review_payloads.pluck('review').pluck('id')).to eq(['review-1'])
|
||||||
|
expect(WebMock).not_to have_requested(:get, 'https://api.appstoreconnect.apple.com/v1/apps/123456789/customerReviews?page=2')
|
||||||
|
end
|
||||||
|
|
||||||
it 'fetches a fresh cached token for each request' do
|
it 'fetches a fresh cached token for each request' do
|
||||||
first_token_service = instance_double(AppStoreConnect::TokenService, token: 'first-token')
|
first_token_service = instance_double(AppStoreConnect::TokenService, token: 'first-token')
|
||||||
second_token_service = instance_double(AppStoreConnect::TokenService, token: 'second-token')
|
second_token_service = instance_double(AppStoreConnect::TokenService, token: 'second-token')
|
||||||
@@ -141,8 +179,8 @@ RSpec.describe AppStoreConnect::Client do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#update_review_response' do
|
describe '#create_or_update_review_response' do
|
||||||
it 'updates an existing response' do
|
it 'creates or updates a response for a review' do
|
||||||
stub_request(:post, 'https://api.appstoreconnect.apple.com/v1/customerReviewResponses')
|
stub_request(:post, 'https://api.appstoreconnect.apple.com/v1/customerReviewResponses')
|
||||||
.with(
|
.with(
|
||||||
body: {
|
body: {
|
||||||
@@ -168,7 +206,7 @@ RSpec.describe AppStoreConnect::Client do
|
|||||||
headers: { 'Content-Type' => 'application/json' }
|
headers: { 'Content-Type' => 'application/json' }
|
||||||
)
|
)
|
||||||
|
|
||||||
response = described_class.new(channel: channel).update_review_response('review-1', 'Updated response')
|
response = described_class.new(channel: channel).create_or_update_review_response('review-1', 'Updated response')
|
||||||
|
|
||||||
expect(response['id']).to eq('response-1')
|
expect(response['id']).to eq('response-1')
|
||||||
end
|
end
|
||||||
|
|||||||
Reference in New Issue
Block a user