Skip to content
Snippets Groups Projects
  1. Aug 02, 2018
  2. Jul 30, 2018
  3. Jul 29, 2018
  4. Jul 26, 2018
  5. Jul 19, 2018
  6. Jul 11, 2018
  7. Jul 09, 2018
  8. Jul 02, 2018
  9. Jun 26, 2018
  10. Jun 07, 2018
    • Sean McGivern's avatar
      Fix some N+1s when calculating notification recipients · 0206476a
      Sean McGivern authored
      First N+1: we may have loaded a user's notification settings already, but not
      have loaded their sources. Because we're iterating through, we'd potentially
      load sources that are completely unrelated, just because they belong to this
      user.
      
      Second N+1: we do a separate query for each user who could be subscribed to or
      unsubcribed from the target. It's actually more efficient in this case to get
      all subscriptions at once, as we will need to check most of them.
      
      We can fix both by the slightly unpleasant means of checking IDs manually,
      rather than object equality.
      0206476a
  11. May 31, 2018
  12. May 22, 2018
  13. May 21, 2018
  14. May 17, 2018
  15. May 16, 2018
    • Dylan Griffith's avatar
    • Dylan Griffith's avatar
      846f73b5
    • Jan Provaznik's avatar
      Changed order of include · 32e22468
      Jan Provaznik authored
      32e22468
    • Jan Provaznik's avatar
      Delete remote uploads · 7da3b2cd
      Jan Provaznik authored
      ObjectStore uploader requires presence of associated `uploads` record
      when deleting the upload file (through the carrierwave's after_commit
      hook) because we keep info whether file is LOCAL or REMOTE in `upload`
      object.
      
      For this reason we can not destroy uploads as "dependent: :destroy" hook
      because these would be deleted too soon. Instead we rely on
      carrierwave's hook to destroy `uploads` in after_commit hook.
      
      But in before_destroy hook we still have to delete not-mounted uploads
      (which don't use carrierwave's destroy hook). This has to be done in
      before_Destroy instead of after_commit because `FileUpload` requires
      existence of model's object on destroy action.
      
      This is not ideal state of things, in a next step we should investigate
      how to unify model dependencies so we can use same workflow for all
      uploads.
      
      Related to #45425
      7da3b2cd
  16. May 14, 2018
  17. May 10, 2018
  18. May 04, 2018
  19. Apr 30, 2018
  20. Apr 28, 2018
    • blackst0ne's avatar
      [Rails5] Fix `params` for DeleteUserWorker · 4a306796
      blackst0ne authored
      This commit fixes the error:
      
      ```
        1) Admin::UsersController DELETE #user with projects deletes the user and their contributions when hard delete is specified
           Failure/Error: Users::DestroyService.new(current_user).execute(delete_user, options.symbolize_keys)
      
           NoMethodError:
             undefined method `symbolize_keys' for "{\"hard_delete\"=>\"true\"}":String
           # ./app/workers/delete_user_worker.rb:8:in `perform'
           # ./lib/gitlab/sidekiq_status/server_middleware.rb:5:in `call'
           # ./config/initializers/forbid_sidekiq_in_transactions.rb:35:in `block (2 levels) in <module:NoEnqueueingFromTransactions>'
           # ./app/models/user.rb:913:in `delete_async'
           # ./app/controllers/admin/users_controller.rb:148:in `destroy'
           # ./lib/gitlab/i18n.rb:50:in `with_locale'
           # ./lib/gitlab/i18n.rb:56:in `with_user_locale'
           # ./app/controllers/application_controller.rb:327:in `set_locale'
           # ./spec/controllers/admin/users_controller_spec.rb:28:in `block (3 levels) in <top (required)>'
      
      Finished in 6.81 seconds (files took 13.9 seconds to load)
      1 example, 1 failure
      ```
      4a306796
  21. Apr 23, 2018
  22. Apr 19, 2018
    • blackst0ne's avatar
      [Rails5] Fix `User#manageable_groups` · 90716733
      blackst0ne authored
      In `arel 7.0` (`7.1.4` version is used for rails5) there were introduced
      some changes that break our code in the `User#manageable_groups` method.
      
      The problem is that `arel_table[:id].in(Arel::Nodes::SqlLiteral)` generates
      wrong `IN ()` construction. The selection for `IN` is missing:
      
      => "\"namespaces\".\"id\" IN (0)"
      
      That caused such spec errors for the `rails5` branch:
      
      ```
      4) User groups with child groups #manageable_groups does not include duplicates if a membership was added for the subgroup
          Failure/Error: expect(user.manageable_groups).to contain_exactly(group, subgroup)
      
            expected collection contained:  [#<Group id:232 @GROUP29>, #<Group id:234 @group29/group30>]
            actual collection contained:    []
            the missing elements were:      [#<Group id:232 @GROUP29>, #<Group id:234 @group29/group30>]
          # ./spec/models/user_spec.rb:699:in `block (5 levels) in <top (required)>'
          # ./spec/spec_helper.rb:188:in `block (2 levels) in <top (required)>'
          # /var/lib/gems/2.3.0/gems/rspec-retry-0.4.6/lib/rspec/retry.rb:112:in `block in run'
          # /var/lib/gems/2.3.0/gems/rspec-retry-0.4.6/lib/rspec/retry.rb:101:in `loop'
          # /var/lib/gems/2.3.0/gems/rspec-retry-0.4.6/lib/rspec/retry.rb:101:in `run'
          # /var/lib/gems/2.3.0/gems/rspec-retry-0.4.6/lib/rspec_ext/rspec_ext.rb:12:in `run_with_retry'
          # /var/lib/gems/2.3.0/gems/rspec-retry-0.4.6/lib/rspec/retry.rb:30:in `block (2 levels) in setup'
      ```
      
      This commit changes `User#manageable_groups` in the way to drop the usage of
      `Arel::Nodes::SqlLiteral` and adds usage of raw SQL query.
      This change should be updated when we're migrated to Rails 5.2 because arel
      was fixed in `9.0.0` (which is used in Rails 5.2).
      90716733
  23. Apr 18, 2018
    • Yorick Peterse's avatar
      Revert the addition of goldiloader · 6f292eaa
      Yorick Peterse authored
      This reverts the addition of the "goldiloader" Gem and all use of it.
      While this Gem is very promising it's causing a variety of problems on
      GitLab.com due to it eager-loading too much data in places where we
      don't expect/can handle this. At least for the time being this means we
      have to go back to manually fixing N+1 query problems, but at least
      those should not cause a negative impact on availability.
      Verified
      6f292eaa
  24. Apr 09, 2018
  25. Apr 06, 2018
  26. Apr 05, 2018
  27. Apr 04, 2018
Loading