Add approval service listener #168

syncrou · 2019-03-21T19:45:23Z

Adds the Approval Request Listener including a bin file for development environments.

Kafka Published Topics (Events) Are as follows: ( at create of the request )

    :payload  => {
      :request_id => '1'
    }

When the request is approved

    :payload  => {
      :request_id => '1'
      :decision =>"approved", 
      :reason"=>"System approved"
    }

syncrou · 2019-03-21T19:48:19Z

@lindgrenj6 please review

lib/approval_request_listener.rb

eclarizio

Waiting for specs to submit full review

lib/approval_request_listener.rb

eclarizio

Looks good to me 👍

lindgrenj6

LGTM!

app/models/approval_request.rb

mkanoor · 2019-03-26T18:50:22Z

bin/approval_request_listener

+require "bundler/setup"
+require "approval_request_listener"
+
+queue_host = ENV["QUEUE_HOST"] || "localhost"


@syncrou can these 4 lines be wrapped into a function and reused here and in config/initializers/approval_request_listener maybe as a class method

In this case it would only be queue_host and queue_port which should be moved to a generic place where all environmental variables are set. We have the same problem with the service order listener. I created this #174 to handle that in a future PR.

lib/approval_request_listener.rb

Handle denied and approved requests Add specs around the listener

mkanoor · 2019-03-26T21:43:00Z

app/models/order_item.rb

@@ -40,7 +40,7 @@ def set_defaults
  end

  def update_message(level, message)
-    self.updated_at = DateTime.now
+    self.updated_at = Time.zone.now


@syncrou This should be in GMT correct?

It will be if we set the rails app config default time to UTC
That's what the zone method does.

Though it may be that by default.

Yeah, default is UTC - http://patrikonrails.com/posts/2

Shouldn't the update_at field get updated automatically at record saving time?

@bzwei @syncrou This is creating a child object the parent will not get automatically updated.

spec/lib/approval_request_listener_spec.rb

mkanoor

We need to account for errors and tenancy which can be done in a separate PR.

bzwei · 2019-03-26T21:53:22Z

app/models/order_item.rb

@@ -40,7 +40,7 @@ def set_defaults
  end

  def update_message(level, message)
-    self.updated_at = DateTime.now
+    self.updated_at = Time.zone.now


Shouldn't the update_at field get updated automatically at record saving time?

config/initializers/approval_request_listener.rb

lib/approval_request_listener.rb

A progress message only has value if there is an order attached to it Since we have now order we are now logging the full payload to the logger so we would have the data available to us in case there was no record found with the request id in ApprovalRequest

miq-bot · 2019-03-27T19:15:54Z

Checked commits syncrou/catalog-api@2cb1cc9~...9846d10 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
5 files checked, 0 offenses detected
Everything looks fine. 🍪

…onsole-errors Fix tests console errors.

syncrou assigned eclarizio, mkanoor and bzwei Mar 21, 2019

syncrou requested review from bzwei, eclarizio and mkanoor March 21, 2019 19:54

syncrou assigned gmcculloug and mkanoor and unassigned eclarizio, mkanoor and bzwei Mar 21, 2019

syncrou added the enhancement New feature or request label Mar 21, 2019

mkanoor reviewed Mar 22, 2019

View reviewed changes

lib/approval_request_listener.rb Outdated Show resolved Hide resolved

bzwei reviewed Mar 22, 2019

View reviewed changes

lib/approval_request_listener.rb Outdated Show resolved Hide resolved

lindgrenj6 reviewed Mar 25, 2019

View reviewed changes

lib/approval_request_listener.rb Outdated Show resolved Hide resolved

lindgrenj6 reviewed Mar 25, 2019

View reviewed changes

lib/approval_request_listener.rb Outdated Show resolved Hide resolved

lindgrenj6 reviewed Mar 25, 2019

View reviewed changes

lib/approval_request_listener.rb Outdated Show resolved Hide resolved

eclarizio reviewed Mar 25, 2019

View reviewed changes

syncrou added 2 commits March 25, 2019 14:09

Added approval request listener

2cb1cc9

Add initializer and bin approval listener

0415e1e

syncrou force-pushed the add_approval_service_listener branch from 35fd5de to 9006c0d Compare March 25, 2019 18:16

lindgrenj6 reviewed Mar 25, 2019

View reviewed changes

lib/approval_request_listener.rb Outdated Show resolved Hide resolved

syncrou force-pushed the add_approval_service_listener branch from 9006c0d to c580166 Compare March 26, 2019 14:47

syncrou marked this pull request as ready for review March 26, 2019 14:47

syncrou requested review from bzwei, mkanoor and eclarizio March 26, 2019 14:47

syncrou force-pushed the add_approval_service_listener branch 2 times, most recently from edb5064 to 97d9de4 Compare March 26, 2019 15:25

syncrou force-pushed the add_approval_service_listener branch 2 times, most recently from d697ec3 to d988ac7 Compare March 26, 2019 17:51

syncrou mentioned this pull request Mar 26, 2019

[WIP] Process approval messaging #57

Closed

eclarizio approved these changes Mar 26, 2019

View reviewed changes

syncrou force-pushed the add_approval_service_listener branch from d988ac7 to f97698a Compare March 26, 2019 18:04

lindgrenj6 approved these changes Mar 26, 2019

View reviewed changes

mkanoor reviewed Mar 26, 2019

View reviewed changes

app/models/approval_request.rb Outdated Show resolved Hide resolved

mkanoor reviewed Mar 26, 2019

View reviewed changes

lib/approval_request_listener.rb Outdated Show resolved Hide resolved

syncrou force-pushed the add_approval_service_listener branch from f97698a to 68cb4f1 Compare March 26, 2019 20:19

mkanoor reviewed Mar 26, 2019

View reviewed changes

lib/approval_request_listener.rb Outdated Show resolved Hide resolved

Update threaded listener

f992122

Handle denied and approved requests Add specs around the listener

syncrou force-pushed the add_approval_service_listener branch 2 times, most recently from cff7658 to 9adc58c Compare March 26, 2019 21:40

mkanoor reviewed Mar 26, 2019

View reviewed changes

spec/lib/approval_request_listener_spec.rb Outdated Show resolved Hide resolved

mkanoor approved these changes Mar 26, 2019

View reviewed changes

bzwei reviewed Mar 26, 2019

View reviewed changes

syncrou force-pushed the add_approval_service_listener branch from 9adc58c to 11db84e Compare March 27, 2019 12:57

bzwei requested changes Mar 27, 2019

View reviewed changes

syncrou force-pushed the add_approval_service_listener branch 2 times, most recently from f606cb6 to 6adbdc6 Compare March 27, 2019 19:04

syncrou force-pushed the add_approval_service_listener branch from 6adbdc6 to 9846d10 Compare March 27, 2019 19:13

syncrou requested a review from bzwei March 27, 2019 19:24

bzwei approved these changes Mar 27, 2019

View reviewed changes

gmcculloug merged commit daa356d into RedHatInsights:master Mar 27, 2019

gmcculloug pushed a commit to gmcculloug/catalog-api that referenced this pull request Nov 8, 2019

Merge pull request RedHatInsights#168 from Hyperkid123/remove-tests-c…

ba6114c

…onsole-errors Fix tests console errors.

Fryguy unassigned mkanoor Nov 15, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add approval service listener #168

Add approval service listener #168

syncrou commented Mar 21, 2019 •

edited

Loading

syncrou commented Mar 21, 2019

eclarizio left a comment

eclarizio left a comment

lindgrenj6 left a comment

mkanoor Mar 26, 2019

syncrou Mar 26, 2019

mkanoor Mar 26, 2019

syncrou Mar 26, 2019

syncrou Mar 26, 2019

syncrou Mar 26, 2019

bzwei Mar 26, 2019

mkanoor Mar 27, 2019

mkanoor left a comment

bzwei Mar 26, 2019

miq-bot commented Mar 27, 2019

Add approval service listener #168

Add approval service listener #168

Conversation

syncrou commented Mar 21, 2019 • edited Loading

syncrou commented Mar 21, 2019

eclarizio left a comment

Choose a reason for hiding this comment

eclarizio left a comment

Choose a reason for hiding this comment

lindgrenj6 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkanoor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miq-bot commented Mar 27, 2019

syncrou commented Mar 21, 2019 •

edited

Loading