Merge pull request #12464 from rubhanazeem/new-watchlist-api

Implement New Watchlist API

src/api/app/controllers/person_controller.rb

@@ -10,7 +10,8 @@ class PersonController < ApplicationController
   skip_before_action :extract_user, only: [:command, :register]
   skip_before_action :require_login, only: [:command, :register]
 
-  before_action :set_user, only: [:post_userinfo, :change_my_password]
+  before_action :set_user, only: [:post_userinfo, :change_my_password, :get_watchlist, :put_watchlist]
+  before_action :check_user_belongs_feature_flag, only: [:get_watchlist, :put_watchlist]
 
   def show
     @list = if params[:prefix]
@@ -134,6 +135,34 @@ def put_userinfo
     render_ok
   end
 
+  def get_watchlist
+    if @user
+      authorize @user, :check_watchlist?
+
+      render xml: @user.render_axml(render_watchlist_only: true)
+    else
+      @errorcode = 404
+      @summary = 'Requested non-existing user'
+      render_error(status: @errorcode)
+    end
+  end
+
+  def put_watchlist
+    if @user
+      authorize @user, :check_watchlist?
+    else
+      @errorcode = 404
+      @summary = 'Requested non-existing user'
+      render_error(status: @errorcode) && return
+    end
+
+    xml = Xmlhash.parse(request.raw_post)
+    ActiveRecord::Base.transaction do
+      update_new_watchlist(@user, xml)
+    end
+    render_ok
+  end
+
   class NoPermissionToGroupList < APIError
     setup 401, 'No user logged in, permission to grouplist denied'
   end
@@ -193,16 +222,21 @@ def internal_register
   end
 
   def update_watchlist(user, xml)
-    new_watchlist = []
-    xml.get('watchlist').elements('project') do |e|
-      new_watchlist << e['name']
-    end
+    # Keeping the backwards compatibility
+    if Flipper.enabled?(:new_watchlist, User.session)
+      update_new_watchlist(user, xml)
+    else
+      new_watchlist = []
+      xml.get('watchlist').elements('project') do |e|
+        new_watchlist << e['name']
+      end
 
-    new_watchlist.map! do |name|
-      WatchedProject.find_or_create_by(project: Project.find_by_name!(name), user: user)
+      new_watchlist.map! do |name|
+        WatchedProject.find_or_create_by(project: Project.find_by_name!(name), user: user)
+      end
+      user.watched_projects.replace(new_watchlist)
+      Rails.cache.delete(['watched_project_names', user])
     end
-    user.watched_projects.replace(new_watchlist)
-    Rails.cache.delete(['watched_project_names', user])
   end
   private :update_watchlist
 
@@ -262,7 +296,32 @@ def change_password(login, password)
 
   private
 
+  def update_new_watchlist(user, xml)
+    if xml.get('watchlist').empty?
+      projects = [xml.get('project')].flatten
+      packages = [xml.get('package')].flatten
+      requests = [xml.get('request')].flatten
+    else
+      projects = xml.get('watchlist').elements('project')
+      packages = xml.get('watchlist').elements('package')
+      requests = xml.get('watchlist').elements('request')
+    end
+
+    watchables = []
+    watchables << projects.map { |proj| Project.find_by(name: proj['name']) }
+    watchables << packages.map { |pkg| Package.find_by_project_and_name(pkg['project'], pkg['name']) }
+    watchables << requests.map { |req| BsRequest.find_by(number: req['number']) }
+    user.watched_items.clear
+    watchables.flatten.compact.each do |item|
+      user.watched_items.create!(watchable: item)
+    end
+  end
+
   def set_user
     @user = User.find_by(login: params[:login])
   end
+
+  def check_user_belongs_feature_flag
+    raise NotFoundError unless Flipper.enabled?(:new_watchlist, User.session)
+  end
 end

src/api/app/models/user.rb

@@ -373,9 +373,9 @@ def to_axml(_opts = {})
     render_axml
   end
 
-  def render_axml(watchlist = false)
+  def render_axml(watchlist = false, render_watchlist_only: false)
     # CanRenderModel
-    render_xml(watchlist: watchlist)
+    render_xml(watchlist: watchlist, render_watchlist_only: render_watchlist_only)
   end
 
   def home_project_name

src/api/app/policies/user_policy.rb

@@ -6,4 +6,8 @@ def update?
   def show?
     user.can_modify_user?(record)
   end
+
+  def check_watchlist?
+    record.login == User.session!.login || User.admin_session?
+  end
 end

src/api/app/queries/watchlist_finder.rb

@@ -0,0 +1,17 @@
+class WatchlistFinder
+  def initialize(relation = WatchedItem.all)
+    @relation = relation
+  end
+
+  def watchlist_projects(user)
+    @relation.includes(:watchable).where(user: user, watchable_type: 'Project').collect(&:watchable)
+  end
+
+  def watchlist_packages(user)
+    @relation.includes(:watchable).where(user: user, watchable_type: 'Package').collect(&:watchable)
+  end
+
+  def watchlist_requests(user)
+    @relation.includes(:watchable).where(user: user, watchable_type: 'BsRequest').collect(&:watchable)
+  end
+end

src/api/app/views/models/_user.xml.builder

@@ -1,25 +1,31 @@
-xml.person do
-  xml.login(my_model.login)
-  xml.email(my_model.email)
-  realname = my_model.realname
-  unless realname.nil?
-    realname.toutf8
-    xml.realname(realname)
-  end
-  xml.owner(userid: my_model.owner.login) if my_model.owner
-  xml.state(my_model.state)
+if render_watchlist_only
+  render partial: 'person/watchlist', locals: { builder: xml, my_model: my_model }
+else
+  xml.person do
+    xml.login(my_model.login)
+    xml.email(my_model.email)
+    realname = my_model.realname
+    unless realname.nil?
+      realname.toutf8
+      xml.realname(realname)
+    end
+    xml.owner(userid: my_model.owner.login) if my_model.owner
+    xml.state(my_model.state)
 
-  my_model.roles.global.each do |role|
-    xml.globalrole(role.title)
-  end
+    my_model.roles.global.each do |role|
+      xml.globalrole(role.title)
+    end
 
-  xml.ignore_auth_services(my_model.ignore_auth_services) if my_model.is_admin?
+    xml.ignore_auth_services(my_model.ignore_auth_services) if my_model.is_admin?
 
-  # Show the watchlist only to the user for privacy reasons
-  if watchlist
-    xml.watchlist do
-      my_model.watched_projects.each do |wp|
-        xml.project(name: wp.project.name)
+    # Show the watchlist only to the user for privacy reasons
+    if Flipper.enabled?(:new_watchlist, User.session) && watchlist
+      render partial: 'person/watchlist', locals: { builder: xml, my_model: my_model }
+    elsif watchlist
+      xml.watchlist do
+        my_model.watched_projects.each do |wp|
+          xml.project(name: wp.project.name)
+        end
       end
     end
   end

src/api/app/views/person/_watchlist.xml.builder

@@ -0,0 +1,11 @@
+builder.watchlist do
+  WatchlistFinder.new.watchlist_projects(my_model).each do |project|
+    builder.project(name: project.name)
+  end
+  WatchlistFinder.new.watchlist_packages(my_model).each do |package|
+    builder.package(name: package.name, project: package.project.name)
+  end
+  WatchlistFinder.new.watchlist_requests(my_model).each do |request|
+    builder.request(number: request.number)
+  end
+end

src/api/config/routes/api_routes.rb

@@ -31,6 +31,8 @@
     get 'person/:login' => 'person#get_userinfo', constraints: cons
     put 'person/:login' => 'person#put_userinfo', constraints: cons
     post 'person/:login' => 'person#post_userinfo', constraints: cons
+    get 'person/:login/watchlist' => 'person#get_watchlist', constraints: cons
+    put 'person/:login/watchlist' => 'person#put_watchlist', constraints: cons
 
     ### /group
     controller :group do

src/api/spec/controllers/person_controller_spec.rb

@@ -113,12 +113,33 @@
     let(:xml) do
       <<-XML_DATA
         <userinfo>
-          <realname>test name</realname>
-          <email>test@test.de</email>
+          <realname>test-user</realname>
+          <email>test-user@email.com</email>
+          <watchlist>
+            <project name="test-proj"/>
+            <package name="test-pkg" project="test-proj"/>
+          </watchlist>
         </userinfo>
       XML_DATA
     end
 
+    context 'when watchlist is available in xml' do
+      let(:test_user) { create(:confirmed_user, login: 'test-user', email: 'test-user@email.com') }
+      let(:project) { create(:project, name: 'test-proj') }
+
+      before do
+        create(:package, project: project, name: 'test-pkg')
+        Flipper.enable_actor(:new_watchlist, admin_user)
+        login admin_user
+        put :put_userinfo, params: { login: test_user.login, format: :xml }, body: xml
+      end
+
+      it "adds projects and packages to user's watchlist" do
+        expect(test_user.watched_items.count).to eq(2)
+        expect(test_user.watched_items.collect(&:watchable)).to include(project)
+      end
+    end
+
     context 'when in LDAP mode' do
       before do
         stub_const('CONFIG', CONFIG.merge('ldap_mode' => :on))
@@ -174,4 +195,59 @@
       end
     end
   end
+
+  describe 'GET #get_watchlist' do
+    context 'user logged-in' do
+      let(:xml) do
+        <<-XML_DATA
+          <watchlist>
+            <project name="test-proj"/>
+            <package name="test-pkg" project="test-proj"/>
+          </watchlist>
+        XML_DATA
+      end
+
+      before do
+        project = create(:project, name: 'test-proj')
+        package = create(:package, project: project, name: 'test-pkg')
+        Flipper.enable_actor(:new_watchlist, user)
+        user.watched_items.create(watchable: project)
+        user.watched_items.create(watchable: package)
+        login user
+      end
+
+      subject! { get :get_watchlist, params: { login: user.login } }
+
+      it 'returns watchlist' do
+        expect(Xmlhash.parse(response.body)).to eq(Xmlhash.parse(xml))
+      end
+    end
+  end
+
+  describe 'PUT #put_watchlist' do
+    context 'creates watchlist' do
+      let(:xml) do
+        <<-XML_DATA
+          <watchlist>
+            <project name="test-proj"/>
+            <package name="test-pkg" project="test-proj"/>
+          </watchlist>
+        XML_DATA
+      end
+
+      let!(:project) { create(:project, name: 'test-proj') }
+      let!(:package) { create(:package, project: project, name: 'test-pkg') }
+
+      before do
+        Flipper.enable_actor(:new_watchlist, user)
+        login user
+        put :put_watchlist, params: { login: user.login, format: :xml }, body: xml
+      end
+
+      it "adds projects and packages to user's watchlist" do
+        expect(user.watched_items.count).to eq(2)
+        expect(user.watched_items.collect(&:watchable)).to include(project, package)
+      end
+    end
+  end
 end