Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Icon showing if file/folder is restricted + tied to a userform submission #1066

Closed
37 tasks done
brynwhyman opened this issue Apr 7, 2020 · 23 comments
Closed
37 tasks done

Comments

@brynwhyman
Copy link

brynwhyman commented Apr 7, 2020

Overview

Provide a clear visual indicator in the Files area to indicate the restricted access of folders and files.

I.e whether they are in the public asset store and as a result are accessible by anonymous website visitors, or if they in the protected asset store and as a result are protected to at least logged-in users only.
NOTE: This work does not include an indication for Folder being associated with a form, this piece has been removed from this issue.

This would look to solve the following opportunities:

  • Before modifying files or folders, I want to make sure they are not associated with form submissions, which may be protected.
  • I can't see which files were uploaded via form submissions so I treat them as I would any other file.

ACs

  • Clearly shows whether a file/folder has restricted access
  • Files indicate they are userform submissions
  • If userform submissions are in the public asset store, then admins have an indication of that fact. Steve: we've logic to based only on permissions, not asset store
  • Ensure the visual cues work well with cues which already exist within the interface already like the Draft badge and the From submission iconography
  • Dev docs have been updated Steve: we've decided there is no need to update dev docs specifically
  • The resulting performance of any change is not significantly different vs. the performance present now for the CMS user. Note, this should be quantified by the Developer.
  • Icons have been added to the most effective and friendly places
    • Icons are rendered in the file manager list/thumbnail view
    • Icons are rendered in the file manager details view
    • Icons are rendered in the file manager breadcrumbs or title
    • Icons are rendered in the file search results view
    • Works correctly at different breakpoints desktop/tablet/mobile
    • There is a way for user of the CMS to find out what the iconography is communicating

Sub-tasks - design

Designs

  • Design icons set for restricted access/public
  • Identify places in which these icons need to be used
  • In flight design review of complete functionality
  • Designs added to DSM/Invision to be consumed by devs
  • Notes added to document designs and approach, and any outdated designs are removed. Everything merged into Abstract
  • Moved to Update userdocs for file status icons silverstripe-userforms#971 Create userhelp content to align with the final experience:

Sub-tasks - dev

  • Investigate performance implications of multiple getVisibility() and canView() checks in the CMS
  • Investigate location used for graphql/json queries and populating response with 'visibility':
    • list
    • details
    • breadcrumbs
    • folder title
    • search
  • pop an icon on for form submissions while at it
  • deploy to a demo server for pux to have a play with it
  • Quick investigation of why the backend of the file manager is so slow in general, see if there are any potential quick wins
  • Implement all design feedback

PRs

Cucumber tests

Accessibility

  • Ran axe, reported no issues related to this work
  • WCAG 2.0 checklist
    • "Provide text alternatives for non-text content" - doing this with reactstrap/bootstrap tooltips instead of html title attribute
    • "Accessible by keyboard only" - cannot tab on to icons, though this is probably a good thing. This is consistent with cannot tab onto draft badge. Pressing tab goes to the next file/folder instead of 'inside' the file/folder element
@brynwhyman
Copy link
Author

brynwhyman commented Apr 7, 2020

What we need to investigate further, make decisions on:

  1. How many states are there for these icons? Is it just for protected files? For protected and unprotected? Or do we need to have something additional for 'this inherits permissions and is protected'?
  2. Are there different icons/ indicators for files and folders?
  3. If this icon was present and the file is moved where the protected state changes, it would be a good UX to warn users that the permission structure will change. This could be raised as a separate issue.
  4. What are the performance implications of this? We might need a spike to investigate this.
  5. Do we need different icons for the grid and list views of the Files area?
  6. What indicators go on files/folders that might otherwise be protected? I.e does a draft file also require a protected icon?
  7. Would this icon be brought into the edit view of the file/ folder?
  8. Could this indicator be considered for a wider use of how to indicate what's hidden from website visitors

@clarkepaul
Copy link
Contributor

Some thoughts:

  • form file, extra secure [secure icon] [submission icon]
  • form file, standard cms security [submission icon]
  • file (non-form), extra secure [secure icon]
  • file (non-form), standard cms security

Not sure about the folders

  • form submission folder, extra secure [secure icon?] [submission icon?]
  • form submission folder, standard cms security [submission icon?]

@clarkepaul
Copy link
Contributor

I think this solution doesn't resolve the opportunity of

Before modifying files or folders, I want to make sure they are not associated with forms or submissions, which may be protected.

We've reworded it slightly to have slightly more emphasis to the protected state but maybe it doesn't aim to resolve the opportunity enough still.

@emteknetnz
Copy link
Member

emteknetnz commented Apr 8, 2020

Tested updated the graphql for doing 50x checks on files

Uploaded over 50 files into a folder, max per page in the File Manager is 50

There's already a File->getVisibility() which returns "public" or "protected". It reads the filesystem.

Added another option is a newly added canViewAnonymous() it may make more sense in some combination of a files modified state and permissions. It used the db rather than the filesystem, so it is much faster than getVisility() - see benchmarks in comment below

assets File.php

    public function canViewAnonymous(): bool
    {
        return Member::actAs(null, function(): bool {
            return $this->canView();
        });
    }

asset-admin FileTypeCreator.php

    public function fields()
    {
            ...
            'visibility' => [
                'type' => Type::string(),
            ],
            'canViewAnonymous' => [
                'type' => Type::boolean(),
            ],

asset-admin fileFragments.js

  fragment FileFields on File {
    ...
    visibility
    canViewAnonymous
  }

@emteknetnz
Copy link
Member

emteknetnz commented Apr 9, 2020

graphql - fileFragments.js

visibilty

  • getVisibility()
  • works on Files
  • but not Folders - function exists though since Folders don't really exist in filesystem
  • will need to use canViewAnonymous() instead

draft

  • used for the existing draft badge (orange dot) in the file manger list view
  • FileTypeCreator->resolveDraftField()
    -> $object->isOnDraftOnly()
    -> Versioned->isOnDraftOnly()
    -> $this->isOnDraft() && !$this->isPublished()

@emteknetnz
Copy link
Member

emteknetnz commented Apr 12, 2020

Benchmark of getVisibility() vs canViewAnonymous() @ 10,000 repetitions on vagrant

tl;dr database canViewAnonymous() is much faster then filesystem getVisibility()

10,000 repetitions:

Published file

  • getVisibility() - 4.4 seconds
  • canViewAnonymous() - 1.2 seconds

Draft-only file

  • getVisibility() - 4.8 seconds
  • canViewAnonymous() - 2.1 seconds

Published file in a Folder with LoggedInUsers permission (so file is in protected asset store)

  • getVisibility() - 4.7 seconds
  • canViewAnonymous() - 1.8 seconds

Note: all the getVisibility() results are all functionally the same since they are agnostic to canView() permissions, difference is just margin of error

Code:

protected function init()
{
    parent::init();

    /** @var File $file */
    Versioned::set_reading_mode('Stage.Stage');

    $file = File::get()->byID(68);
    $repetitions = 10000;

    $this->testFileGetVisibility($file, $repetitions);
    $this->testFileCanViewAnonymous($file, $repetitions);
}

private function testFileGetVisibility(File $file, int $repetitions)
{
    // @10000 repetitions - published file
    // 5,057,039, 3,947,287, 4,542,077
    // avg: 4,515,468 - 4.5 seconds

    // @10000 repetitions - draft-only file
    // 4,325,805, 5,586,085, 4,544,136
    // avg: 4,818,675 - 4.8 seconds

    // @10000 repetitions - published file in folder with LoggedInUsers permissions
    // 4,708,242, 4,719,711, 4,714,409
    // avg: 4,714,121 - 4.7 seconds

    for ($i = 0; $i < $repetitions; $i++) {
        $file->getVisibility();
    }
}

private function testFileCanViewAnonymous(File $file, int $repetitions)
{
    // @10000 repetitions - published file
    // 1,209,930, 1,512,013, 1,064,074
    // avg: 1,262,006 - 1.2 seconds

    // @10000 repetitions - draft-only file
    // 2,112,515, 2,093,698, 2,110,813
    // avg: 2,105,675 - 2.1 seconds

    // @10000 repetitions - published file in folder with LoggedInUsers permissions
    // 1,883,311, 1,782,117, 1,839,047
    // avg: 1,834,825 - 1.8 seconds

    for ($i = 0; $i < $repetitions; $i++) {
        $file->canViewAnonymous();
    }
}

@clarkepaul
Copy link
Contributor

I assume we are not catering to the scenario of a file archive being used, meaning we don't need to check whether permissions are retained once a file is returned from the archive.

@brynwhyman
Copy link
Author

I think this solution doesn't resolve the opportunity of...

@clarkepaul I agree. What about an update to:

This would look to solve the following opportunity:
Before modifying files or folders, I want to be clear of the current protected state of these items, so I do not make changes that accidentally expose files that should be protected to the public.

@brynwhyman
Copy link
Author

Also a quick note: can we please watch out that tasks or comments related to 'an icon for form submissions' are kept in issue silverstripe/silverstripe-userforms#950 and not in this issue? While there's overlap, we should make sure to keep these conversations separate.

I bet the way the current opportunity is phrased doesn't help...

@brynwhyman
Copy link
Author

Note that we've had discussions where performance has been significantly impacted in other areas of the CMS, some sites have wanted to see functionality have the ability to opt out. I don't think we should default to this, but worth noting this has been considered before...

@emteknetnz
Copy link
Member

emteknetnz commented Apr 16, 2020

After viewing a demo of use a green/blue icon that shows if a file is in the public/protected asset store, it was determined that this is a very confusing method of displaying 'protected'

The core issue is that a file can be in the protected store by either:

  • draft-only
  • permissions

The asset store abstraction is not something CMS users are normally aware of, so the green/blue icon was very confusing to people

After this, we went through and exercise of making the protected icon derive purely from permissions instead of which asset store the file was in

@emteknetnz
Copy link
Member

emteknetnz commented Apr 16, 2020

Showing a protected icon based on permissions is fraught because some websites allow customers to create an account. The majority of sites do not.

This does mean that the 'LoggedInUsers' permission:

  • On the majority of sites means 'User with CMS access'
  • On a minority of sites it means 'Customer (who we can assume lacks CMS access)'

We cannot assume, nor can we reliably automatically determine, which of these two models a site follows

What this means is we are unable to automatically derive business logic for what a permission based 'protected' state means

In order to get the protected icons working, we would need to add something like a CMS editable option in Site Settings such as 'Customers can log in to an account', which we'd probably default to 'off'.

Site administrators on sites where customer can login, would then need to manually set this in order for the protected icons to display correctly

This does not strike me as a particularly good outcome

@emteknetnz
Copy link
Member

emteknetnz commented Apr 16, 2020

Extra note about canView() checks on DataObjects, if we do decide to progress this further, and we want to accommodate sites where customers can log in, then we'll need to feed the canViewAnonymous() function with a member written to the database:

public function canViewAnonymous(): bool
{
    $customerCanLogin = Config::get() .. 'customer_can_log_in'
    $member = $customerCanLogin
        ? $member = Member::create() ... $member->write() // <<-- must write()
         : null;
    return Member::actAs($member, function(): bool {
        return $this->canView();
    });
}

This is because all the canView() permission checks the ORM does actually require a member in the database

What this means is there will be a member with a strange email such as 'zz_member_with_no_cms_access' floating around in the Member table

This will probably be very confusing to CMS users who see it in the Security section

@brynwhyman brynwhyman changed the title Visual indicator showing if file/folder is protected or public Icon showing if file/folder is restricted + tied to a userform submission Apr 27, 2020
@brynwhyman brynwhyman modified the milestones: Sprint 57, Sprint 58 Apr 27, 2020
@emteknetnz emteknetnz self-assigned this Apr 28, 2020
@clarkepaul
Copy link
Contributor

clarkepaul commented Apr 28, 2020

The places we might need to consider placing both icons for restricted and form submission the icons. All placements should have a tooltip to educate users of what the icons mean.
Designs to accompany this list

Priority placements:

  • File folder
  • File folder edit view
  • Thumb file
  • List view
  • File edit view
  • Toolbar north breadcrumbs
  • Folder dropdown within userform file upload field
  • Modal for selecting a folder or creating new folder

Med priority placements:

  • Uploadfield
  • Search results (might inherit list view icons?)

Low priority placements:

  • Move file dropdown, folder dropdowns
  • TinyMCE images
  • Campaigns
  • Blocks which have uploadfield or files (should inherit if done above?)
  • History - files are not stored typically when archived, often it's not a good experience as there isn't even a record that a file was ever there. Archived forms with submissions show as a blank space where files once where.

Excluding:

  • Any other module, for example; blog, subsites, advanced workflow, document converter, DMS...

@clarkepaul
Copy link
Contributor

clarkepaul commented Apr 29, 2020

I would like to propose not using the "Submission" icon to signify a folder has an association with a form/submission.

There are four main approaches for using an icon to show an association between a folder and a form/submissions:

  1. Show icon if a form is currently linked to a folder
    [-] Can have form submission in folders without the icon
    [-] Icon is being used to communicate a slightly different thing than when it's used on a file (e.g. file connected to submission, folder connected to form)
    [+/-] Icon indicates the link between folder and a form only but not about its contents

  2. Show icon if the folder was created by a form for submission storage (has preset restricted access permissions)
    [-] If the folder is changed at a later stage you end up with random form folders
    [-] Can't turn the icon off, if the folder is disconnected from the form
    [-] Can't move the icon to the newly selected folder
    Ruling out as option

  3. If a folder contains form submissions
    [+] More factual that a folder contains submissions,
    [+] Consistent use of iconography indicating the file is linked to submission (rather than a folder linking to a form)
    [-] It doesn't indicate where additional submissions are going to be placed, they could be going to a folder which does not contain any other submission yet so doesn't have an icon on the folder

  4. Don't show any icon for indicating its form/submission related
    [+] Not sending mixed messages with iconography that means different things
    [+] We don't have two icons on folders, we already have the restricted access icon which is probably more meaningful and important
    [-] Doesn't communicate that the folder is associated with a user-defined form(s)

Each of the above approaches has its downsides and plus sides.

There isn't space for two icons on the folder but there is in other places...
image.png

@emteknetnz
Copy link
Member

Option 1 and Option 3 (what the demo currently has) are both confusing for the same reason - we're using the same icon on the folder as we are on the file.

Option 1

  • On a folder - icon mean this folder is the destination for a userform upload field
  • On a file - icon mean this file is a submitted file

Option 3

  • On a folder - icon means this contains submitted files
  • On a file - icon means this is a submitted file

Option 2 is just confusing, not really an option IMO

Option 4 is obviously the easy solution, non-confusing though at the same time non-informative

If we were to implement either 1 or 3 then it seems like we'd need a new icon

Option 1 seems nice from a "I just created a new user form" point of view

Option 3 is generally better though IMO

  • From a legacy 'I've had userforms on my site for the last 5 years" point of view, it makes it easier to navigate around the filesystem and find where all the submitted files are, including cases where the upload folder was changed halfway through a userforms life. A lot of our customers will have had userforms spanning several years so there's a lot of legacy
  • It also largely does what option 1 does in terms of showing which folders have userform uploadfields pointed at them, as all it takes is a single submitted file to make the icon appear

So my recommendation is we stick with option 3 and create a new icon

@emteknetnz
Copy link
Member

Note: I would be OK with option 4 if we instead created a report separate from the file manager showing the location of Folders/Files of submitted files. I think that would do a better job of helping people audit their website for submitted files than file manager icons alone would.

@clarkepaul
Copy link
Contributor

clarkepaul commented Apr 29, 2020

Agree that opt 2 is a no-go so crossed it out.
I really don't want to create yet again another icon which is a variation of a form—that just creates more clutter and confusion.

I think the thing the main thing we are actually trying to communicate is that this folder is being used by "something" so be careful with it, regardless if its a form, the carousel, the company staff list of profile pictures. In this case, the icon would be a link icon showing there is a relationship of which I can't tell you what the relationship is but don't delete it (also not very informative). Multiple forms can be associated with a single folder so you probably don't want to list all the relationships in the tooltip itself.

Either way, we don't have room on the folder to display two icons so I'm recommending not showing any form related icon there. The folder itself is an icon so with the addition of two more icons we're just overloading things. Open to bright ideas though.

@emteknetnz
Copy link
Member

emteknetnz commented Apr 29, 2020 via email

@clarkepaul
Copy link
Contributor

An update based on this morning discussion... We are not going to indicate that a folder is associated to a form from the files area as part of this issue (no submission icon on folders).

This could still happen as a separate issue if either of the above options (1 & 3) get selected to go further with, so as part of this issue we are going for option 1 (don't show submission icon on folders).

Side note, if we went further with option 1 indicating that a folder is connected to a userform we could possibly use a different coloured folder icon (for example we could use purple or grey colour). This is pretty abstract so it would need to be reinforced with a tooltip.

@emteknetnz
Copy link
Member

Demo feedback

  • Try pulling in jquery bootstrap tooltips to use use of File details panel (Garion)
  • Put Folder 'metadata' e.g icon in top row (different from File metadata)
  • Modified badge wrong - remove orange background

@brynwhyman
Copy link
Author

Re the new points that have been added on a11y:

"Accessible by keyboard only" - cannot tab on to icons, though this is probably a good thing. This is consistent with cannot tab onto draft badge. Pressing tab goes to the next file/folder instead of 'inside' the file/folder element

I'd guess that the fact that you can't tab to the 'draft' badge should probably be seen as a bug and not necessarily as functionality we're trying to copy as it limits how some should ideally access this information. @clarkepaul should I create a new issue to log this as a problem? I know that eventually we'd want to see that draft label as a reactstrap component too.

@clarkepaul
Copy link
Contributor

I think the experience we'd want to use is tabbing navigates you from one file to another, return selects that file (takes you to edit panel), spacebar to check an item, and arrows allows you into the thumb tile area to be able to navigate to the info tooltips. escape is step back up so the thumb has focus again. We'd really need to standardise a keyboard approach within the CMS as things can get complicated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants