Skip to content

[PB-6503] Add new sharings#1996

Open
TamaraFinogina wants to merge 15 commits into
masterfrom
add_new_sharings
Open

[PB-6503] Add new sharings#1996
TamaraFinogina wants to merge 15 commits into
masterfrom
add_new_sharings

Conversation

@TamaraFinogina

@TamaraFinogina TamaraFinogina commented Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Description

This PR switches to new public sharing version

Related Pull Requests

Server change

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.
  • QA Passed

Testing Process

  1. When a file or a folder that has never been shared previously is publicly shared on this environment, then it can be viewed and downloaded on production. The sharing version should be inxt-v3.
  2. When a file or a folder that has been shared previously is publicly shared on this environment, then it still can be viewed and downloaded on production. The sharing verision should be inxt-v2.
  3. When a file or a folder is shared with a non-user via user-invitation, then it work as before.
  4. When a file or a folder is shared with existing user, then it work as before.
  5. When user is invited to a workspace, then it works as before.

Additional Notes

@TamaraFinogina TamaraFinogina self-assigned this Jun 17, 2026
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown

Deploying drive-web with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5dc75a0
Status: ✅  Deploy successful!
Preview URL: https://801ac28c.drive-web.pages.dev
Branch Preview URL: https://add-new-sharings.drive-web.pages.dev

View logs

@TamaraFinogina TamaraFinogina marked this pull request as ready for review June 17, 2026 14:31
@TamaraFinogina TamaraFinogina requested a review from sg-gs June 17, 2026 14:32
larryrider
larryrider previously approved these changes Jun 18, 2026
View reviewed changes
CandelR
CandelR reviewed Jun 18, 2026
View reviewed changes

@CandelR CandelR left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

waiting server changes to test it ⚠️

@TamaraFinogina TamaraFinogina marked this pull request as draft June 18, 2026 14:43
@TamaraFinogina TamaraFinogina marked this pull request as ready for review June 18, 2026 16:23
@TamaraFinogina

TamaraFinogina commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

@CandelR Server is ready, we can test it

@TamaraFinogina TamaraFinogina changed the title [_] Add new sharings [PB-6503] Add new sharings Jun 18, 2026
CandelR
CandelR reviewed Jun 19, 2026
View reviewed changes
Comment thread src/app/share/services/share.service.ts
token: string;
}

const NEW_SHARING_VERSION = 'inxt-v3';

@CandelR CandelR Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

nit: It would be better if both the server and the website had a single source of truth, because if it changes again one day and this logic isn’t taken into account, it could break

@TamaraFinogina TamaraFinogina Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, but I'm not sure where to put it. SDK types?

@sg-gs sg-gs Jun 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It should be the backend, the SDK is only a reflect of it @TamaraFinogina

Comment thread src/app/share/services/share.service.ts Outdated
if (encriptedMnemonic) {
const ownerMnemonic = await decryptMnemonic(encriptedMnemonic);
if (ownerMnemonic) mnemonic = ownerMnemonic;
if (ownerMnemonic) localStorageService.set(LocalStorageItem.UserMnemonic, ownerMnemonic);

@CandelR CandelR Jun 19, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

rewrite own mnemonic with another users one not seems good idea. This can lead in decript or encription errors if in some point we use this one.
Maybe I’m getting this wrong, could you confirm it?

@TamaraFinogina TamaraFinogina Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Old logic was:

Do sharing (which requires a mnemonic, so without a mnemonic it does something strange), then check if the encrypted mnemonic is given and the user is invited; if so, use it to decrypt the code.

Now:

I moved code decryption inside the function, so I need to have the mnemonic in advance. The only place that calls getPublicShareLink with an encrypted mnemonic is onCopyLink in case isAdvancedShareItem exists. I think it's sharing with an invitation. So, it should be the same. But I'm a bit confused about how it was working before: if the mnemonic was undefined prior to setting it - it should have failed share creation; if the mnemonic of the invited user was already set at that point - why double setting? I'm lost.

@TamaraFinogina TamaraFinogina Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@CandelR Fixed, should be exactly as before

@TamaraFinogina TamaraFinogina requested a review from xabg2 June 19, 2026 10:09
@sonarqubecloud

sonarqubecloud Bot commented Jun 19, 2026

Copy link
Copy Markdown

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
82.1% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@TamaraFinogina TamaraFinogina requested a review from CandelR June 19, 2026 12:14
sg-gs
sg-gs reviewed Jun 19, 2026
View reviewed changes
Comment thread src/app/share/services/share.service.test.ts
expect(spyDecrypt).not.toHaveBeenCalled();
});

test('When encrypted code changes, decrypt code', async () => {

@sg-gs sg-gs Jun 19, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When does this happen? I am not sure

@TamaraFinogina TamaraFinogina Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@sg-gs, it's when we share the same file again. If the file was already shared, the server will not change sharings, and to access the file via the link, we need the same code as in the server records, so we decrypt it.

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

Reviewers

@sg-gs sg-gs sg-gs left review comments

@larryrider larryrider Awaiting requested review from larryrider larryrider is a code owner

@xabg2 xabg2 Awaiting requested review from xabg2

@CandelR CandelR Awaiting requested review from CandelR CandelR is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

@TamaraFinogina TamaraFinogina

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@TamaraFinogina @larryrider @sg-gs @CandelR