Skip to content
This repository was archived by the owner on Jun 20, 2025. It is now read-only.

Add migration to Intercom #1

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ulizko
Copy link

@ulizko ulizko commented Sep 6, 2017

No description provided.

@@ -3,6 +3,8 @@

get '/', to: 'folders#index'

post '/tickets/:id/export', to: 'tickets#export', as: :export_ticket
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are rails-like resource routes http://hanamirb.org/guides/routing/restful-resources/

ticket = TicketRepository.new.find(params[:id])
ticket.migrate
redirect_to '/'
end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some notification like "Exported started" would be great


def messages
MessageRepository.new.by_ticket(id)
end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like associations. There is support of assoications in Hanami: http://hanamirb.org/guides/1.0/models/associations/

Gemfile Outdated
@@ -7,6 +7,8 @@ gem 'hanami-bootstrap'
gem 'omniauth-google-oauth2'
gem 'warden'
gem 'groovehq'
gem 'intercom'
gem 'sucker_punch', '~> 2.0'
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's wrong with Sidekiq?

require 'intercom'

module Services
class Intercom
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it recommended way to implement service objects? Ain't Hanami Entities better choice?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants