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

[PB-2424] Feat: working with ancestors to display shared item details #1403

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

evillalba94
Copy link

@evillalba94 evillalba94 commented Dec 24, 2024

Description

Adapted a NEW VERSION ENDPOINT of ancestors for Workspaces.
Fixed a bug when displaying details of items in the drive and shared folders for both individual and workspace.

Related Issues

Related Pull Requests

Checklist

  • Changes have been tested locally.
  • Unit tests have been written or updated as necessary.
  • The code adheres to the repository's coding standards.
  • Relevant documentation has been added or updated.
  • No new warnings or errors have been introduced.
  • SonarCloud issues have been reviewed and addressed.

How Has This Been Tested?

  1. Have a user as owner of a workspace.
  2. Invite a user to the workspace
  3. Create a team and add both (owner and invited)
  4. Share a folder with subfolders and files with the team
  5. Go to the shared section
  6. View the details of the files and folders inside the shared folder

Additional Notes

  • When opening the details and looking at the location, it was set to show only /Shared to avoid showing folders that the owner user does not want to reveal or share as a security measure.
    image

  • The share icon has been removed from internal folders and files. The icon only appears on the item that was initially shared. This is to allow the user to know which item they need to remove share access from. However, if there are folders or files that have been shared separately and are inside another shared folder, then the icon will be displayed.
    image
    File with change: src/app/share/views/SharedLinksView/components/SharedItemList.tsx

@evillalba94 evillalba94 added the enhancement New feature or request label Dec 24, 2024
@evillalba94 evillalba94 self-assigned this Dec 24, 2024
@evillalba94 evillalba94 requested a review from CandelR as a code owner December 24, 2024 16:05
Copy link

vercel bot commented Dec 24, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
drive-web ❌ Failed (Inspect) Dec 27, 2024 6:10pm

@evillalba94 evillalba94 marked this pull request as draft December 24, 2024 18:25
Comment on lines 138 to 148
let location = '/';
if (item.view === 'Shared') {
location += translate('sideNav.shared');
if (item.isFolder) {
location += getPathName.length > 0 ? '/' + getPathName.shift() : '';
}
} else {
if (item.isFolder) {
getPathName.pop();
}
location += rootPathName + (getPathName.length > 0 ? '/' + getPathName.join('/') : '');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separate this logic into functions for ease of understanding and maintenance

Copy link

Deploying drive-web with  Cloudflare Pages  Cloudflare Pages

Latest commit: 12e60b2
Status:⚡️  Build in progress...

View logs

@evillalba94 evillalba94 marked this pull request as ready for review December 27, 2024 18:10
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Comment on lines +126 to +156
function getSharedLocation(item: DriveItemDetails, ancestorPathNames: string[]): string {
let location = translate('sideNav.shared');
if (item.isFolder && ancestorPathNames.length > 0) {
location += '/' + ancestorPathNames.shift();
}
return location;
}

function getRegularLocation(item: DriveItemDetails, ancestorPathNames: string[]): string {
if (item.isFolder) {
ancestorPathNames.pop();
}
return item.view + (ancestorPathNames.length > 0 ? '/' + ancestorPathNames.join('/') : '');
}

function getLocation(item: DriveItemDetails, ancestors: DriveItemData[]): string {
const itemViewName = item.view;
const ancestorPathNames = ancestors.map((ancestor) => getItemPlainName(ancestor)).reverse();
ancestorPathNames.shift(); // Remove root parent

let location = '/';

if (itemViewName === 'Shared') {
location += getSharedLocation(item, ancestorPathNames);
return location;
}

location += getRegularLocation(item, ancestorPathNames);
return location;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember to add tests for these functions, you will probably need to extract them from this file to add the unit tests

@CandelR
Copy link
Contributor

CandelR commented Jan 8, 2025

@jvedrson check the comments in order to finish the task

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request ready-for-preview
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants