Merge pull request #128 from datacite/feature_remove_providerid

Feature remove provider_id references

app/controllers/reports_controller.rb

@@ -35,7 +35,7 @@ def index
                  elsif params[:year].present?
                    Report.where(year: params[:year])
                  elsif params[:client_id].present?
-                   Report.where(client_id: params[:client_id])
+                   Report.where(user_id: params[:client_id])
                  else
                    Report.all
                  end
@@ -93,7 +93,7 @@ def create
     @report = Report.where(created_by: params[:report_header].dig(:created_by)).
       where(month: get_month(params[:report_header].dig(:reporting_period, "begin_date"))).
       where(year: get_year(params[:report_header].dig(:reporting_period, "begin_date"))).
-      where(client_id: params.merge(@user_hash)[:client_id]).
+      where(user_id: params.merge(@user_hash)[:user_id]).
       first
     exists = @report.present?
 
@@ -120,7 +120,7 @@ def set_report
   end
 
   def set_user_hash
-    @user_hash = { client_id: current_user.client_id, provider_id: current_user.provider_id }
+    @user_hash = { user_id: current_user.uid }
   end
 
   def validate_monthly_report

app/models/ability.rb

@@ -9,10 +9,8 @@ def initialize(user)
 
     if user.role_id == "staff_admin"
       can :manage, :all
-    elsif user.role_id == "provider_admin" && user.provider_id.present?
-      can [:create, :update, :read], Report, :provider_id => user.provider_id
-    elsif user.role_id == "client_admin" && user.client_id.present?
-      can [:create, :update, :read], Report, :client_id => user.client_id
+    elsif user.role_id == "client_admin" && user.uid.present?
+      can [:create, :update, :read], Report, :user_id => user.uid
     end
   end
 end

app/models/concerns/authenticable.rb

@@ -73,16 +73,20 @@ def get_payload(uid: nil, user: nil)
         "email" => user.contact_email
       }
 
-      if uid.include? "."
-        payload.merge!({
-          "provider_id" => uid.split(".", 2).first,
-          "client_id" => uid
-        })
-      elsif uid != "admin"
-        payload.merge!({
-          "provider_id" => uid
-        })
-      end
+      # if uid.include? "."
+      #   payload.merge!({
+      #     "provider_id" => uid.split(".", 2).first,
+      #     "client_id" => uid
+      #   })
+      # elsif uid != "admin"
+      #   payload.merge!({
+      #     "provider_id" => uid
+      #   })
+      # end
+
+      # payload.merge!({
+      #   "client_id" => uid
+      # })
 
       payload
     end
@@ -121,8 +125,8 @@ def encode_auth_param(username: nil, password: nil)
     # generate JWT token
     def generate_token(attributes={})
       payload = {
-        uid:  attributes.fetch(:uid, "0000-0001-5489-3594"),
-        name: attributes.fetch(:name, "Josiah Carberry"),
+        uid:  attributes.fetch(:uid, "datacite.datacite"),
+        name: attributes.fetch(:name, "staff"),
         email: attributes.fetch(:email, nil),
         provider_id: attributes.fetch(:provider_id, nil),
         client_id: attributes.fetch(:client_id, nil),
@@ -147,16 +151,20 @@ def get_payload(uid: nil, user: nil)
         "email" => user.contact_email
       }
 
-      if uid.include? "."
-        payload.merge!({
-          "provider_id" => uid.split(".", 2).first,
-          "client_id" => uid
-        })
-      elsif uid != "admin"
-        payload.merge!({
-          "provider_id" => uid
-        })
-      end
+      # # if uid.include? "."
+      # #   payload.merge!({
+      # #     "provider_id" => uid.split(".", 2).first,
+      # #     "client_id" => uid
+      # #   })
+      # # elsif uid != "admin"
+      # #   payload.merge!({
+      # #     "provider_id" => uid
+      # #   })
+      # # end
+
+      # payload.merge!({
+      #   "client_id" => uid
+      # })
 
       payload
     end

app/models/concerns/cacheable.rb

@@ -5,50 +5,50 @@ module Cacheable
     def cached_reports_count(id)
       if Rails.application.config.action_controller.perform_caching
         Rails.cache.fetch("cached_reports_count/#{id}", expires_in: 24.hours) do
-          Report.where(client_id: id).count
+          Report.where(user_id: id).count
         end
       else
-        Report.where(client_id: id).count
+        Report.where(user_id: id).count
       end
     end
 
     def cached_client_count
       if Rails.application.config.action_controller.perform_caching
         Rails.cache.fetch("cached_client_count", expires_in: 24.hours) do
-          Report.group(:client_id).count
+          Report.group(:user_id).count
         end
       else
-        Report.group(:client_id).count
+        Report.group(:user_id).count
       end
     end
 
     def cached_created_by_count(id, options={})
       if Rails.application.config.action_controller.perform_caching
         Rails.cache.fetch("created_by_count/#{id}", expires_in: 24.hours) do
-          Report.where(client_id: id).group(:created_by).count
+          Report.where(user_id: id).group(:created_by).count
         end
       else
-        Report.where(client_id: id).group(:created_by).count
+        Report.where(user_id: id).group(:created_by).count
       end
     end
 
     def cached_year_count(id)
       if Rails.application.config.action_controller.perform_caching
         Rails.cache.fetch("cached_year_count/#{id}", expires_in: 24.hours) do
-          Report.where(client_id: id).group(:year).count
+          Report.where(user_id: id).group(:year).count
         end
       else
-        Report.where(client_id: id).group(:year).count
+        Report.where(user_id: id).group(:year).count
       end
     end
 
     def cached_report_id_count(id, options={})
       if Rails.application.config.action_controller.perform_caching
         Rails.cache.fetch("cached_report_id_count/#{id}", expires_in: 24.hours) do
-          Report.where(client_id: id).group(:release).count
+          Report.where(user_id: id).group(:release).count
         end
       else
-        Report.where(client_id: id).group(:release).count
+        Report.where(user_id: id).group(:release).count
       end
     end
 

app/models/report.rb

@@ -16,14 +16,14 @@ class Report < ApplicationRecord
   include Queueable
 
   # attr_accessor :month, :year, :compressed
-  validates_presence_of :report_id, :created_by, :client_id, :provider_id, :created, :reporting_period
+  validates_presence_of :report_id, :created_by, :user_id, :created, :reporting_period
   validates_presence_of :report_datasets, if: :normal_report?
   validates_format_of :created_by, with: /[-\._;()\/:a-zA-Z0-9\*~\$\=]+\z/, on: :create
 
   # , :report_datasets
   validates :uid, uniqueness: true
   validates :validate_sushi, sushi: { presence: true }, if: :normal_report?
-  attr_readonly :created_by, :month, :year, :client_id, :report_id, :uid
+  attr_readonly :created_by, :month, :year, :user_id, :report_id, :uid
 
   # serialize :exceptions, Array
   before_validation :set_uid, on: :create

app/serializers/report_serializer.rb

@@ -18,7 +18,7 @@ def report_header
     {
       :report_name => object.report_name,
       :report_id => object.report_id,
-      :client_id => object.client_id,
+      :client_id => object.user_id,
       :year => object.year,
       :month => object.month,
       :release => object.release,

db/migrate/20200709092430_remove_provider_id_from_reports.rb

@@ -0,0 +1,5 @@
+class RemoveProviderIdFromReports < ActiveRecord::Migration[5.2]
+  def change
+    remove_column :reports, :provider_id, :string
+  end
+end

db/migrate/20200713101137_change_client_id_from_reports.rb

@@ -0,0 +1,5 @@
+class ChangeClientIdFromReports < ActiveRecord::Migration[5.2]
+  def change
+    rename_column :reports, :client_id, :user_id
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2019_09_24_074609) do
+ActiveRecord::Schema.define(version: 2020_07_13_101137) do
 
   create_table "active_storage_attachments", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 ROW_FORMAT=DYNAMIC", force: :cascade do |t|
     t.string "name", null: false
@@ -73,8 +73,7 @@
   create_table "reports", options: "ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 ROW_FORMAT=DYNAMIC", force: :cascade do |t|
     t.string "report_name", default: "Dataset Report"
     t.string "report_id"
-    t.string "client_id", null: false
-    t.string "provider_id", null: false
+    t.string "user_id", null: false
     t.string "release", default: "RD1"
     t.string "created"
     t.string "created_by"

spec/concerns/authenticable_spec.rb

@@ -7,7 +7,7 @@
   describe 'decode_token' do
     it "has name" do
       payload = subject.decode_token(token)
-      expect(payload["name"]).to eq("Josiah Carberry")
+      expect(payload["name"]).to eq("staff")
     end
 
     it "empty token" do
@@ -23,7 +23,7 @@
 
   describe 'encode_token' do
     it "with name" do
-      token = subject.encode_token("name" => "Josiah Carberry")
+      token = subject.encode_token("name" => "staff")
       expect(token).to start_with("eyJhbG")
     end
 

spec/factories/default.rb

@@ -2,8 +2,7 @@
 FactoryBot.define do
   factory :report do
     sequence(:report_id) { |n| "dsr-12hd-zt#{n}" }
-    client_id { "datacite.datacite" }
-    provider_id { "datacite" }
+    user_id { "datacite.datacite" }
     sequence(:created_by) { |n| "datacite#{n}" }
     created { "2020-03-02" }
     reporting_period  { { "begin_date": "2018-03-01", "end_date": "2018-03-31" } }
@@ -50,8 +49,7 @@
 
   factory :resolution_report do
     sequence(:report_id) { |n| "dsr-12hd-zt#{n}" }
-    client_id { "datacite.datacite" }
-    provider_id { "datacite" }
+    user_id { "datacite.datacite" }
     sequence(:created_by) { |n| "datacite#{n}" }
     created { "2020-03-02" }
     reporting_period  { { "begin_date": "2018-03-01", "end_date": "2018-03-31" } }

spec/models/abilities_spec.rb

@@ -0,0 +1,53 @@
+require "rails_helper"
+require "cancan/matchers"
+
+describe User, type: :model do
+  let(:token) { User.generate_token }
+  let(:user) { User.new(token) }
+  let(:report) { create(:report, user_id: "datacite.datacite") }
+  let(:report_subset) { create(:report_subset, report_id: report.uid) }
+
+  let(:report_cdl) { create(:report, user_id: "cdl.dash") }
+  let(:report_subset_cdl) { create(:report_subset, report_id: report_cdl.uid) }
+
+  describe "User attributes", order: :defined do
+    it "is valid with valid attributes" do
+      expect(user.name).to eq("staff")
+    end
+  end
+
+  describe "abilities", vcr: true do
+    subject { Ability.new(user) }
+
+    context "when is a staff_admin" do
+      let(:token){ User.generate_token(role_id: "staff_admin") }
+
+      it { is_expected.to be_able_to(:read, user) }
+      it { is_expected.to be_able_to(:read, report) }
+      it { is_expected.to be_able_to(:create, report) }
+      it { is_expected.to be_able_to(:update, report) }
+      it { is_expected.to be_able_to(:destroy, report) }
+    end
+
+    context "when is a client admin" do
+      let(:token){ User.generate_token(role_id: "client_admin", uid: "datacite.datacite") }
+
+      it { is_expected.not_to be_able_to(:read, user) }
+
+      it { is_expected.to be_able_to(:read, report) }
+      it { is_expected.to be_able_to(:create, report) }
+      it { is_expected.to be_able_to(:update, report) }
+      it { is_expected.not_to be_able_to(:destroy, report) }
+
+      it { is_expected.not_to be_able_to(:read, report_cdl) }
+      it { is_expected.not_to be_able_to(:create, report_cdl) }
+      it { is_expected.not_to be_able_to(:update, report_cdl) }
+      it { is_expected.not_to be_able_to(:destroy, report_cdl) }
+
+      # it { is_expected.to be_able_to(:read, report_subset) }
+      # it { is_expected.to be_able_to(:create, report_subset) }
+      # it { is_expected.to be_able_to(:update, report_subset) }
+      # it { is_expected.not_to be_able_to(:destroy, report_subset) }
+    end
+  end
+end

spec/models/user_spec.rb

@@ -11,7 +11,7 @@
       end
 
       it "has name" do
-        expect(user.name).to eq("Josiah Carberry")
+        expect(user.name).to eq("staff")
       end
     end
   end

spec/requests/report_types_spec.rb

@@ -1,7 +1,7 @@
 require 'rails_helper'
 
 describe 'ReportTypes', type: :request do
-  let(:bearer) { User.generate_token(client_id: "datacite.datacite", provider_id: "datacite", role_id: "staff_admin") }
+  let(:bearer) { User.generate_token(uid: "datacite.datacite", role_id: "staff_admin") }
   let(:headers) { {'ACCEPT'=>'application/json', 'CONTENT_TYPE'=>'application/json', 'Authorization' => 'Bearer ' + bearer}}
 
   # describe 'GET /reports' do

spec/requests/reports_spec.rb

@@ -1,7 +1,7 @@
 require "rails_helper"
 
 describe "Reports", type: :request do
-  let(:bearer) { User.generate_token(exp: Time.now.to_i + 300, client_id: "datacite.datacite", provider_id: "datacite", role_id: "staff_admin") }
+  let(:bearer) { User.generate_token(exp: Time.now.to_i + 300, uid: "datacite.datacite", role_id: "staff_admin") }
   let(:headers) { { "ACCEPT" => "application/json", "CONTENT_TYPE" => "application/json", "Authorization" => "Bearer " + bearer } }
 
   describe "GET /reports" do
@@ -64,9 +64,9 @@
     let(:params) { file_fixture("report_3.json").read }
     context "when the request is valid" do
       before { post "/reports", params: params, headers: headers }
+
 
       it "creates a report" do
-        # puts json
         expect(json.dig("report", "report-header", "report-name")).to eq("dataset report")
         expect(response).to have_http_status(201)
       end
@@ -98,7 +98,7 @@
     end
 
     context "index filter by client_id" do
-      let!(:bearer_ext) { User.generate_token(client_id: "datacite.demo", provider_id: "datacite", role_id: "staff_admin") }
+      let!(:bearer_ext) { User.generate_token(uid: "datacite.demo",role_id: "staff_admin") }
       let!(:headers_ext) { { "ACCEPT" => "application/json", "CONTENT_TYPE" => "application/json", "Authorization" => "Bearer " + bearer_ext } }
 
       before { post "/reports", params: params, headers: headers_ext }