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.

Sign up for GitHub

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

Jump to bottom

fix(files_sharing): file request followups #46589

Merged
merged 17 commits into from
Jul 19, 2024
Merged

fix(files_sharing): file request followups #46589

merged 17 commits into from
Jul 19, 2024

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Jul 17, 2024 •
edited
Loading

Sans.titre.mp4

Todo

  • Some design adjusting based on design team request
  • Simplified email validation based on groupware team feedback
  • Use share label as title and destination if provided
  • Prompt guest nickname to upload files in subfolder

Followups

  • Think about name conflicts
  • Add warning when a share without READ gets change with READ
  • Add time left on public page
  • Add list of file requests in Navigation

github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 17, 2024
View reviewed changes
apps/dav/lib/Files/Sharing/FilesDropPlugin.php Outdated
* @param View $view
*/
public function setView($view) {
public function setView(View $view) {

Check notice

Code scanning / Psalm

MissingReturnType Note

Method OCA\DAV\Files\Sharing\FilesDropPlugin::setView does not have a return type, expecting void
apps/files_sharing/lib/Listener/LoadPublicFileRequestAuthListener.php Dismissed Show dismissed Hide dismissed
@skjnldsv skjnldsv force-pushed the fix/files_sharing-file-request-followup branch 2 times, most recently from c8e6edd to 45c9298 Compare July 17, 2024 17:47
@skjnldsv skjnldsv self-assigned this Jul 17, 2024
@skjnldsv skjnldsv added this to the Nextcloud 30 milestone Jul 17, 2024
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 17, 2024
View reviewed changes
apps/dav/lib/Files/Sharing/FilesDropPlugin.php Outdated
@@ -16,16 +19,13 @@
*/
class FilesDropPlugin extends ServerPlugin {

/** @var View */
private $view;
private View $view;

Check notice

Code scanning / Psalm

MissingConstructor Note

OCA\DAV\Files\Sharing\FilesDropPlugin has an uninitialized property OCA\DAV\Files\Sharing\FilesDropPlugin::$view, but no constructor
apps/dav/lib/Files/Sharing/FilesDropPlugin.php Dismissed Show dismissed Hide dismissed
apps/dav/lib/Files/Sharing/FilesDropPlugin.php Outdated
}

// We need a valid nickname for file requests
if ($isFileRequest && (!$nickName || trim($nickName) === '')) {

Check notice

Code scanning / Psalm

RiskyTruthyFalsyComparison Note

Operand of type null|string contains type string, which can be falsy and truthy. This can cause possibly unexpected behavior. Use strict comparison instead.
apps/dav/lib/Files/Sharing/FilesDropPlugin.php Outdated
// If this is a file request we need to create a folder for the user
if ($isFileRequest) {
// Check if the folder already exists
if (!$this->view->file_exists($nickName)) {

Check notice

Code scanning / Psalm

RiskyTruthyFalsyComparison Note

Operand of type bool|mixed contains type mixed, which can be falsy and truthy. This can cause possibly unexpected behavior. Use strict comparison instead.
@skjnldsv skjnldsv force-pushed the fix/files_sharing-file-request-followup branch 2 times, most recently from 71f5f64 to cb95dc9 Compare July 18, 2024 06:00
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 18, 2024
View reviewed changes
apps/dav/lib/Files/Sharing/FilesDropPlugin.php Outdated
// If this is a file request we need to create a folder for the user
if ($isFileRequest) {
// Check if the folder already exists
if (!$this->view->file_exists($nickName) === true) {

Check notice

Code scanning / Psalm

RiskyTruthyFalsyComparison Note

Operand of type bool|mixed contains type mixed, which can be falsy and truthy. This can cause possibly unexpected behavior. Use strict comparison instead.
apps/dav/lib/Files/Sharing/FilesDropPlugin.php Outdated
// If this is a file request we need to create a folder for the user
if ($isFileRequest) {
// Check if the folder already exists
if (!$this->view->file_exists($nickName) === true) {

Check notice

Code scanning / Psalm

PossiblyNullReference Note

Cannot call method file_exists on possibly null value
@skjnldsv skjnldsv force-pushed the fix/files_sharing-file-request-followup branch from cb95dc9 to bcc8923 Compare July 18, 2024 06:29
@skjnldsv

This comment was marked as resolved.

Sign in to view
@skjnldsv

This comment was marked as resolved.

Sign in to view
@skjnldsv

This comment was marked as resolved.

Sign in to view
@backportbot backportbot bot mentioned this pull request Jul 18, 2024
Merged
This was referenced Jul 18, 2024
Merged
Merged
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 18, 2024
View reviewed changes
apps/dav/lib/Files/Sharing/FilesDropPlugin.php Outdated
}

// If this is a file request we need to create a folder for the user
if ($isFileRequest && $nickName !== null) {

Check failure

Code scanning / Psalm

RedundantCondition Error

string can never contain null
apps/dav/lib/Files/Sharing/FilesDropPlugin.php
// If this is a file request we need to create a folder for the user
if ($isFileRequest && $nickName !== null) {
// Check if the folder already exists
if (!($this->view->file_exists($nickName) === true)) {

Check notice

Code scanning / Psalm

PossiblyNullReference Note

Cannot call method file_exists on possibly null value
@skjnldsv skjnldsv force-pushed the fix/files_sharing-file-request-followup branch from bcc8923 to 6051047 Compare July 18, 2024 11:07
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 18, 2024
View reviewed changes
apps/dav/lib/Files/Sharing/FilesDropPlugin.php Outdated
}

// If this is a file request we need to create a folder for the user
if ($isFileRequest && $nickName) {

Check failure

Code scanning / Psalm

RedundantCondition Error

Operand of type non-falsy-string is always truthy
@skjnldsv skjnldsv force-pushed the fix/files_sharing-file-request-followup branch 2 times, most recently from a5a34b0 to f5871b1 Compare July 18, 2024 11:37
@skjnldsv skjnldsv removed the 2. developing Work in progress label Jul 18, 2024
@skjnldsv skjnldsv requested a review from artonge July 18, 2024 14:54
github-advanced-security[bot]
github-advanced-security bot found potential problems Jul 18, 2024
View reviewed changes
apps/dav/lib/Files/Sharing/FilesDropPlugin.php Outdated
$shareManager = \OCP\Server::get(IManager::class);
$token = $this->getToken($request);
$share = $shareManager->getShareByToken($token);
if ($share === null) {

Check notice

Code scanning / Psalm

DocblockTypeContradiction Note

OCP\Share\IShare does not contain null
apps/files_sharing/lib/Listener/LoadPublicFileRequestAuthListener.php Outdated
$isFileRequest = false;
try {
$share = $this->shareManager->getShareByToken($token);
if ($share === null) {

Check notice

Code scanning / Psalm

DocblockTypeContradiction Note

OCP\Share\IShare does not contain null
skjnldsv added 9 commits July 18, 2024 19:47
@skjnldsv
fix(files_sharing): file request conditions with link/email global se…
365b647 
…ttings

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
fix(files_sharing): file request use l10n t and n aliases
018e4c0 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
fix(files_sharing): file request add email on focusout
f956d51 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
fix(files_sharing): file request margin and buttons order
eb2cda1 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
fix(files_sharing): improve file request email handling
55d4194 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
fix(files_sharing): use ClipboardText icon and fix trailing-button-click
fec615c 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
fix(files_sharing): file request creation permissions and link copy d…
43ca078 
…isplay

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
feat(files_sharing): use label as title and file drop destination if …
b80c57d 
…available

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
feat(files_sharing): allow uploading to subfolders in File requests
f45d613 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv skjnldsv force-pushed the fix/files_sharing-file-request-followup branch 3 times, most recently from 25911a3 to 98f17a1 Compare July 18, 2024 17:57
skjnldsv added 7 commits July 18, 2024 20:15
@skjnldsv
feat(files_sharing): add public name prompt for files requests
f28d933 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
fix: drop outdated handlebar comments template
7878f1c 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
fix(files_sharing): also allow removing READ permissions on email shares
2c94404 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
fix(files_sharing): improve file request info messages
320af31 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
fix(files_sharing): file request form validation and date component e…
08d3fed 
…vent

Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
fix(files_sharing): file request expiration date timezone
607f0b0 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv
chore: compile assets
481551e 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv skjnldsv force-pushed the fix/files_sharing-file-request-followup branch from 98f17a1 to 481551e Compare July 18, 2024 18:15
@skjnldsv skjnldsv enabled auto-merge July 18, 2024 19:03
@skjnldsv
chore(integration): add file request tests
725736a 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv skjnldsv force-pushed the fix/files_sharing-file-request-followup branch from e578a8c to 725736a Compare July 19, 2024 05:35
@skjnldsv skjnldsv merged commit 0bde47a into master Jul 19, 2024
166 checks passed
@skjnldsv skjnldsv deleted the fix/files_sharing-file-request-followup branch July 19, 2024 07:14
@blizzz blizzz mentioned this pull request Jul 24, 2024
Merged
@joshtrichards joshtrichards mentioned this pull request Aug 19, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@susnux susnux susnux approved these changes

@sorbaugh sorbaugh sorbaugh approved these changes

@icewind1991 icewind1991 Awaiting requested review from icewind1991

@Pytal Pytal Awaiting requested review from Pytal Pytal was automatically assigned from nextcloud/server-frontend

@nfebe nfebe Awaiting requested review from nfebe

@artonge artonge Awaiting requested review from artonge

Assignees

@skjnldsv skjnldsv

Labels
3. to review Waiting for reviews enhancement feature: sharing
Projects
📁 Files team
Status: 🏗️ In progress
Milestone
Nextcloud 30
Development

Successfully merging this pull request may close these issues.
4 participants
@skjnldsv @susnux @artonge @sorbaugh