Adapt API's StatusMessage to the new structure

Handle scope (communication_scope) in status message's XML.

The validation schema considers scope as optional. When the value is not
passed, the default value "all_users" is stored instead.

Add show view to status message and move StatusMessageController related
views to partials.

Adapt tests and docs's XML files.

Co-authored-by: Victor Pereira <vpereira@suse.com>

docs/api/api/status_message.xml

@@ -1,7 +1,11 @@
 <?xml version="1.0" encoding="UTF-8"?>
-<message severity="0">
-  a single sample message. contains some text ...
-  attributes are optional and ignored when sent (put) to the api.
-  the format of this example will be used as a convenient way to put a message.
-</message>
+<status_message>
+  <message>
+    a single sample message. contains some text ...
+    attributes are optional and ignored when sent (put) to the api.
+    the format of this example will be used as a convenient way to put a message.
+  </message>
+  <severity>information</severity>
+  <scope>all_users</scope>
+</status_message>
 

docs/api/api/status_message.xsd

@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<xs:schema xmlns:xs="http://www.w3.org/2001/XMLSchema" elementFormDefault="qualified" attributeFormDefault="unqualified">
+  <xs:element name="status_message">
+    <xs:complexType>
+      <xs:sequence>
+        <xs:element name="message" type="xs:string"></xs:element>
+        <xs:element name="severity" type="xs:string"></xs:element>
+        <xs:element name="scope" type="xs:string" minOccurs="0" default="all_users"></xs:element>
+      </xs:sequence>
+      <xs:attribute name="id" type="xs:int"></xs:attribute>
+    </xs:complexType>
+  </xs:element>
+</xs:schema>

src/api/app/controllers/status_messages_controller.rb

@@ -1,50 +1,37 @@
 class StatusMessagesController < ApplicationController
-  class PermissionDeniedError < APIError
-    setup 403
-  end
-
-  class CreatingMessagesError < APIError; end
+  before_action :require_admin, only: [:create, :destroy]
 
   def index
     @messages = StatusMessage.alive.limit(params[:limit]).order('created_at DESC').includes(:user)
     @count = @messages.size
   end
 
   def show
-    @messages = [StatusMessage.find(params[:id])]
-    @count = 1
-    render :index
+    @message = StatusMessage.find(params[:id])
   end
 
   def create
-    # check permissions
-    unless permissions.status_message_create
-      raise PermissionDeniedError, 'message(s) cannot be created, you have not sufficient permissions'
-    end
-
-    new_messages = Nokogiri::XML(request.raw_post, &:strict).root
-    @messages = []
-    if new_messages.css('message').present?
-      # message(s) are wrapped in outer xml tag 'status_messages'
-      new_messages.css('message').each do |msg|
-        @messages << StatusMessage.create!(message: msg.content, severity: msg['severity'], user: User.session!)
-      end
-    else
-      # TODO: make use of a validator
-      raise CreatingMessagesError, "no message #{new_messages.to_xml}" if new_messages.name != 'message'
-      # just one message, NOT wrapped in outer xml tag 'status_messages'
-      @messages << StatusMessage.create!(message: new_messages.content, severity: new_messages['severity'], user: User.session!)
-    end
-    render :index
+    status_message = StatusMessage.from_xml(validate_status_message)
+
+    authorize status_message
+
+    status_message.save!
+
+    render_ok
   end
 
   def destroy
-    # check permissions
-    unless permissions.status_message_create
-      raise PermissionDeniedError, 'message cannot be deleted, you have not sufficient permissions'
-    end
-
-    StatusMessage.find(params[:id]).delete
+    status_message = StatusMessage.find(params[:id])
+    authorize status_message
+    status_message.delete
     render_ok
   end
+
+  private
+
+  # TODO: make it more robust
+  def validate_status_message
+    Suse::Validator.validate(:status_message, request.raw_post)
+    request.raw_post
+  end
 end

src/api/app/models/status_message.rb

@@ -10,6 +10,17 @@ class StatusMessage < ApplicationRecord
 
   enum severity: { information: 0, green: 1, yellow: 2, red: 3, announcement: 4 }
 
+  # xml: A Nokogiri object
+  def self.from_xml(xml)
+    StatusMessage.create! if xml.blank?
+    doc = Nokogiri::XML(xml, &:strict).root
+    message = doc.css('message').text
+    severity = doc.css('severity').text
+    scope = doc.css('scope').text
+    scope = 'all_users' if scope.blank?
+    StatusMessage.new(message: message, severity: severity, communication_scope: scope, user: User.session!)
+  end
+
   def delete
     self.deleted_at = Time.now
     save

src/api/app/policies/status_message_policy.rb

@@ -0,0 +1,9 @@
+class StatusMessagePolicy < ApplicationPolicy
+  def create?
+    user.is_admin?
+  end
+
+  def destroy?
+    user.is_admin?
+  end
+end

src/api/app/views/status_messages/_status_message.xml.builder

@@ -0,0 +1,7 @@
+builder.status_message(id: status_message.id) do |m|
+  m.message status_message.message
+  m.user status_message.user.login
+  m.severity status_message.severity
+  m.scope status_message.communication_scope
+  m.created_at status_message.created_at
+end

src/api/app/views/status_messages/index.xml.builder

@@ -1,12 +1,3 @@
 xml.status_messages(count: @count) do
-  @messages.each do |msg|
-    xml.message(
-      msg.message,
-      msg_id: msg.id,
-      user: msg.user.login,
-      severity: msg.severity,
-      created_at: msg.created_at,
-      deleted_at: msg.deleted_at
-    )
-  end
+  render(partial: 'status_message', collection: @messages, locals: { builder: xml })
 end

src/api/app/views/status_messages/show.xml.builder

@@ -0,0 +1 @@
+render(partial: 'status_message', locals: { builder: xml, status_message: @message })

src/api/spec/controllers/status_messages_controller_spec.rb

@@ -4,19 +4,21 @@
   render_views
 
   let(:user) { create(:confirmed_user) }
-  let(:status_message) { create(:status_message) }
+  let(:status_message) { create(:status_message, user: user) }
 
   before do
     login user
   end
 
   describe 'GET #show' do
+    before { status_message }
+
     subject! { get :show, params: { id: status_message.id }, format: :xml }
 
     it { is_expected.to have_http_status(:success) }
 
     it 'returns the requested status message' do
-      assert_select 'status_messages[count=1]' do
+      assert_select 'status_message' do
         assert_select 'message', status_message.message
       end
     end
@@ -41,22 +43,18 @@
   describe '#create' do
     let(:request_xml) do
       <<~XML
-        <status_messages>
-          <message severity="green">New message was sent!</message>
-        </status_messages>
+        <status_message>
+          <message>New message was sent!</message>
+          <severity>green</severity>
+          <scope>all_users</scope>
+        </status_message>
       XML
     end
 
     context 'when user is not admin' do
       subject! { post :create, body: request_xml, format: :xml }
 
       it { is_expected.to have_http_status(:forbidden) }
-
-      it "responds with a 'permission_denied' status" do
-        assert_select 'status[code=permission_denied]' do
-          assert_select 'summary', 'message(s) cannot be created, you have not sufficient permissions'
-        end
-      end
     end
 
     context 'when requester is admin' do
@@ -70,13 +68,27 @@
 
       it { is_expected.to have_http_status(:success) }
 
-      it 'responds with the created message' do
-        assert_select 'status_messages' do
-          assert_select 'message', 'New message was sent!'
-        end
+      it { expect(StatusMessage.last.message).to eq('New message was sent!') }
+    end
+
+    context 'create with a wrong XML' do
+      let(:request_xml) do
+        <<~XML
+          <stadus_message>
+            <message>New message was sent!</message>
+            <severity>information</severity>
+          </status_message>
+        XML
       end
+      let(:admin) { create(:admin_user) }
 
-      it { expect(StatusMessage.last.message).to eq('New message was sent!') }
+      before do
+        login admin
+      end
+
+      subject! { post :create, body: request_xml, format: :xml }
+
+      it { is_expected.to have_http_status(:bad_request) }
     end
   end
 
@@ -86,12 +98,6 @@
 
       it { is_expected.to have_http_status(:forbidden) }
       it { expect(status_message.deleted_at).to be_nil }
-
-      it "responds with a 'permission_denied' status" do
-        assert_select 'status[code=permission_denied]' do
-          assert_select 'summary', 'message cannot be deleted, you have not sufficient permissions'
-        end
-      end
     end
 
     context 'when requester is admin' do

src/api/spec/models/status_message_spec.rb

@@ -0,0 +1,28 @@
+require 'rails_helper'
+
+RSpec.describe StatusMessage do
+  let(:admin_user) { create(:admin_user, login: 'admin') }
+
+  describe 'validations' do
+    it { is_expected.to validate_presence_of(:severity) }
+    it { is_expected.to validate_presence_of(:message) }
+  end
+
+  describe '.from_xml' do
+    before do
+      allow(User).to receive(:session!).and_return(admin_user)
+    end
+
+    context 'xml is valid' do
+      let(:xml) { '<status_message id="4"><message>foo</message><severity>information</severity></status_message>' }
+      let(:status_message) { StatusMessage.from_xml(xml) }
+
+      it { expect { status_message }.not_to raise_error }
+      it { expect(status_message).to be_a(StatusMessage) }
+    end
+
+    context 'xml is invalid' do
+      it { expect { StatusMessage.from_xml('') }.to raise_error(ActiveRecord::RecordInvalid) }
+    end
+  end
+end

src/api/test/functional/status_controller_test.rb

@@ -25,18 +25,18 @@ def test_new_message
     post '/status/messages', params: '<whereareyou/>'
     assert_response 400
 
-    post '/status/messages', params: '<messages><message>nada</message></messages>'
+    post '/status/messages', params: '<status_message><message>nada</message></status_message>'
     assert_response 400
-    assert_xml_tag attributes: { code: 'invalid_record' }
+    assert_xml_tag attributes: { code: 'validation_failed' }
 
-    post '/status/messages', params: '<message severity="yellow">I have nothing to say</message>'
+    post '/status/messages', params: '<status_message><message>I have nothing to say</message><severity>yellow</severity></status_message>'
     assert_response :success
 
     # delete it again
     get '/status/messages'
     assert_response :success
     messages = Xmlhash.parse @response.body
-    msg_id = messages.get('message').value('msg_id')
+    msg_id = messages.get('status_message').value('id')
 
     prepare_request_valid_user
     delete "/status/messages/#{msg_id}"