Skip to content
Snippets Groups Projects

Improve how File Lock feature works with nested items

Merged Valery Sizov requested to merge file_lock_nested_locks into master

fixes https://gitlab.com/gitlab-org/gitlab-ee/issues/705

Recent screenshot: Screen_Shot_2016-06-23_at_12.12.20

Previous one joxi_screenshot_1466623505450

Merge request reports

Pipeline #3621880 passed

Pipeline passed for 15e6d78d on file_lock_nested_locks

Approval is optional

Merged by avatar (Feb 23, 2025 6:59am UTC)

Merge details

  • Changes merged into master with 0f21c62b.
  • Deleted the source branch.
  • Auto-merge enabled

Pipeline #3622356 passed

Pipeline passed for 0f21c62b on master

4 environments impacted.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Valery Sizov Added 22 commits:

    Added 22 commits:

    • 6093b529...22cbe843 - 19 commits from branch master
    • 1542f36a - Show nested locks
    • 8a7b635e - Don't show lock controls for nested items
    • 39278d73 - Render file lock icon in file path breadcrumbs
  • Valery Sizov Added 1 commit:

    Added 1 commit:

    • 0cabb88a - address review comments
  • Valery Sizov Added 1 commit:

    Added 1 commit:

    • 33533ed5 - lock by actual locked folder in breadcrumbs
  • Valery Sizov Added 1 commit:

    Added 1 commit:

    • d9cc9585 - revert license stub[ci skip]
  • Valery Sizov Added 1 commit:

    Added 1 commit:

    • 3735a8f0 - minor imrovement
  • Valery Sizov Added 1 commit:

    Added 1 commit:

    • 81588077 - always show lock/unlock controls but disable them if needed[ci skip]
  • Valery Sizov Added 1 commit:

    Added 1 commit:

    • 78b0cf32 - Show lock icon in the end of file name
  • I'm thinking about the different situations for the Lock/Unlock button, and what is the most helpful text for the user. What about:

    • 1 Directory lib/gitlab/ or file lib/gitlab/git.rb

      • 1.1. Locked
        • 1.1.1. Exact lock
          • 1.1.1.1. Can unlock because I locked it
            • "Unlock", enabled, no tooltip
          • 1.1.1.2. Can unlock because I am admin
            • "Unlock", enabled, tooltip Locked by Douwe Maan
          • 1.1.1.3. Cannot unlock (because someone else locked it)
            • "Unlock", disabled, tooltip Locked by Douwe Maan. You do not have permission to unlock this file.
        • 1.1.2. Upstream lock lib/
          • 1.1.2.1. Can unlock (because I locked it or am admin)
            • "Unlock", disabled, tooltip Douwe Maan has a lock on "lib/". Unlock that directory in order to unlock this file.
          • 1.1.2.2. Cannot unlock (because someone else locked it)
            • "Unlock", disabled, tooltip Douwe Maan has a lock on "lib/". You do not have permission to unlock it.
    • 2 Directory lib/gitlab/

      • 2.1. Unlocked
        • 2.1.1. Downstream lock lib/gitlab/git.rb
          • 2.1.1.1. Can unlock (because I locked it or am admin)
            • "Lock", disabled, tooltip This directory cannot be locked while Douwe Maan has a lock on "lib/gitlab/git.rb". Unlock that file in order to proceed.
          • 2.1.1.2. Cannot unlock (because someone else locked it)
            • "Lock", disabled, tooltip This directory cannot be locked while Douwe Maan has a lock on "lib/gitlab/git.rb". You do not have permission to unlock it.
        • 2.1.2. No lock at all
          • 2.1.2.1. Can lock (because I have push access)
            • "Lock", enabled, no tooltip
          • 2.1.2.2. Cannot lock (because I don't have push access)
            • "Lock", disabled, tooltip "You do not have permission to lock this directory."
    • 3 File lib/gitlab/git.rb

      • 3.1. Unlocked
        • 3.1.1. Can lock (because I have push access)
          • "Lock", enabled, no tooltip
        • 3.1.2. Cannot lock (because I don't have push access)
          • "Lock", disabled, tooltip "You do not have permission to lock this file."

    /cc @rspeicher @jschatz1

    Edited by Valery Sizov
  • Author Contributor

    @DouweM I edited your nice comment, sorry for that. I added numbers to be able to discuss some of the items.

  • Author Contributor

    2.1.1.2. I think You tried to say "Cannot lock" it would make sense. @DouweM

    UPDATE: If yes, please update your comment

    UPDATE1: But still, the text is unclear to me.

    Edited by Valery Sizov
  • Author Contributor

    Ah, OK, I got it, it's correct. "Cannot unlock" actually refers to "2.1.1. Downstream lock lib/gitlab/git.rb"

  • Author Contributor

    So I agree with all these items. I built my TODO list according to your schema.

    • Show tooltip for active “Unlock” button when lock is not mine but I have permissions to unlock (case 1.1.1.2.)

    • Support of Downstream locks. (case 2.1.1)

    • Fix text messages according to schema above

    Edited by Valery Sizov
  • @vsizov Yep, the "Cannot unlock" referred to the downstream lock itself :) Todo sounds good!

  • Author Contributor

    @DouweM haha. Case 2.1.1. What if we have more then one downstream lock?. At this time I tool first one. This is logic hell, my brain is blowing up.

  • Author Contributor

    @DouweM 1.1.1.3 and 1.1.2.1 contain "file" in the end of the sentence and I actually removed it from those phrases because we don't know if this is a file or a directory but checking it out would be too silly so it's better to rephrase.

    Edited by Valery Sizov
  • Valery Sizov Added 2 commits:

    Added 2 commits:

    • 5ea22577 - [FileLock] logic hell, first implementation
    • d05bf6a6 - [FileLock] Feature is working but refactoring is still needed
  • Author Contributor
    • Page refresh after click on Lock/Unlock
    • Review comment about @ref replace
    • Specs
    • tree_helper refactoring
    • Think of validation in both rails and database side one more time.
    Edited by Valery Sizov
  • Valery Sizov Added 1 commit:

    Added 1 commit:

    • 92515e35 - [FL] Downstream locks is not searched by default[ci skip]
  • Valery Sizov Added 1083 commits:

    Added 1083 commits:

    • 92515e35...f00c8ede - 1071 commits from branch master
    • 903d1e76 - Show nested locks
    • 8863108a - Don't show lock controls for nested items
    • aca1ce41 - Render file lock icon in file path breadcrumbs
    • 9bfb4af5 - address review comments
    • 7aae1886 - lock by actual locked folder in breadcrumbs
    • 891dc21f - revert license stub[ci skip]
    • 2b4360b1 - minor imrovement
    • f1ff39d5 - always show lock/unlock controls but disable them if needed[ci skip]
    • 426ecafe - Show lock icon in the end of file name
    • 8a8986f2 - [FileLock] logic hell, first implementation
    • a66d0e4e - [FileLock] Feature is working but refactoring is still needed
    • c51b5c15 - [FL] Downstream locks is not searched by default[ci skip]
  • Valery Sizov Added 1 commit:

    Added 1 commit:

    • 5e484326 - update changelog [ci skip]
  • Valery Sizov Added 1 commit:

    Added 1 commit:

    • 2b3a7136 - refactoring tree_breadcrumbs heler
  • Valery Sizov Added 1 commit:

    Added 1 commit:

    • 227289c5 - Page refresh after click on Lock/Unlock[ci skip]
  • Valery Sizov Added 2 commits:

    Added 2 commits:

    • 626169dc - spec for tree helper
    • a8980ed7 - tree_helper spec refactoring
  • Valery Sizov Added 3 commits:

    Added 3 commits:

    • 6df00a39 - tree_helper refactoring
    • e14b8ed3 - TYPO
    • d75ef4ff - specs for path_lock model
  • Valery Sizov Added 1 commit:

    Added 1 commit:

    • c70b97ae - Improve how File Lock feature works with nested items
  • Author Contributor

    @DouweM Please check it out

  • @DouweM re:

    I'm thinking about the different situations for the Lock/Unlock button, and what is the most helpful text for the user. What about:

    That sounds good to me.

  • Valery Sizov Added 4 commits:

    Added 4 commits:

    • c70b97ae...8a7e9852 - 3 commits from branch master
    • e43b804c - Improve how File Lock feature works with nested items
  • Reassigned to @DouweM

  • Valery Sizov Added 1 commit:

    Added 1 commit:

    • ae5731ea - Improve how File Lock feature works with nested items
  • Valery Sizov Added 1 commit:

    Added 1 commit:

    • ae5731ea - Improve how File Lock feature works with nested items
  • Valery Sizov Added 1 commit:

    Added 1 commit:

    • e4f9e2d3 - Improve how File Lock feature works with nested items
  • Douwe Maan
    Douwe Maan @DouweM started a thread on the diff
  • 6 6 - Fix encrypted data backwards compatibility after upgrading attr_encrypted gem. !502
    7 7 - Fix creating MRs on forks of deleted projects. !503
    8 8 - Roll back Grack::Auth to fix Git HTTP SPNEGO. !504
    9 - Improve how File Lock feature works with nested items
  • Douwe Maan Added 1 commit:

    Added 1 commit:

  • Douwe Maan Added 1 commit:

    Added 1 commit:

  • Douwe Maan Added 1 commit:

    Added 1 commit:

  • Douwe Maan Enabled an automatic merge when the build for 15e6d78d succeeds

    Enabled an automatic merge when the build for 15e6d78d succeeds

  • Douwe Maan Status changed to merged

    Status changed to merged

  • Douwe Maan mentioned in commit 0f21c62b

    mentioned in commit 0f21c62b

  • Robert Speicher Removed ~149424 label

    Removed ~149424 label

  • Picked into 8-9-stable-ee, will be in 8.9.4.

  • Douwe Maan mentioned in commit 4ebe197d

    mentioned in commit 4ebe197d

  • Douwe Maan Mentioned in commit 4ebe197d

    Mentioned in commit 4ebe197d

  • Douwe Maan Mentioned in commit 0f21c62b

    Mentioned in commit 0f21c62b

  • Please register or sign in to reply
    Loading