Update `getWithChildren` to use stela #720

slifty · 2025-09-15T20:16:31Z

This PR updates getWithChildren to use stela.

This function is not used in many places right now -- in particular I was only able to reliably trigger it by viewing the Apps section of the navigation.

We picked this one because it will be used for unlisted shares.

Related to #683

src/app/shared/services/api/folder.repo.ts

src/app/shared/services/api/record.repo.ts

src/app/shared/services/data/data.service.ts

codecov · 2025-09-25T15:33:24Z

Codecov Report

❌ Patch coverage is 22.58065% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.04%. Comparing base (2e200fa) to head (3c5811e).
⚠️ Report is 8 commits behind head on main.

Files with missing lines	Patch %	Lines
src/app/shared/services/api/folder.repo.ts	11.11%	24 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #720      +/-   ##
==========================================
- Coverage   45.16%   45.04%   -0.12%     
==========================================
  Files         370      370              
  Lines       11284    11309      +25     
  Branches     1860     1862       +2     
==========================================
- Hits         5096     5094       -2     
- Misses       6013     6042      +29     
+ Partials      175      173       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

aasandei-vsp

From my point of view, taking into account that I'm still getting used to the code base, it looks good.

slifty · 2025-09-29T15:06:34Z

@aasandei-vsp Thank you, also I just wanted to underscore how accurate your review was / please do continue to flag things like that even if in this case we didn't end up making every change!

aasandei-vsp · 2025-09-30T08:30:03Z

@aasandei-vsp Thank you, also I just wanted to underscore how accurate your review was / please do continue to flag things like that even if in this case we didn't end up making every change!

I promise to do this and push back as well whenever I have a strong opinion about smth and back it up with arguments. I feel this is the only way to actually get to a healthy codebase.

cecilia-donnelly

I had some pending comments - I will leave those here since @aasandei-vsp has already done such a thorough review!

cecilia-donnelly · 2025-09-19T19:02:58Z

src/app/shared/services/api/folder.repo.ts

+	stelaFolder.children ??= [];
+	const childFolderVOs = stelaFolder.children
+		.filter((child): child is StelaFolder => !isStelaRecord(child))
+		.map(convertStelaFolderToFolderVO);


Do we want to have a limit on how deeply we recurse? I suppose not since we need folders to work all the way down. Maybe a better question is: "does this recurse all the way down the children of a folder and do we need it to do that?"

We could prevent a second layer just in defensive spirit -- but this also begs the question about the data stela returns (I assume folders in stela are shallow?)

PHP doesn't provide full depth right (i.e. our components are already assuming child folders need to be loaded?) <-- I will check.

cecilia-donnelly · 2025-09-19T19:06:25Z

src/app/shared/services/api/folder.repo.ts

+		thumbURL2000: stelaFolder.thumbnailUrls['2000'],
+		thumbDT: stelaFolder.displayTimestamp,
+		thumbnail256: stelaFolder.thumbnailUrls['256'],
+		thumbnail256CloudPath: stelaFolder.thumbnailUrls['256'],


Do you know why we have both of these?

It's in the VO (not sure if both are actually used but I suppose therein lies the ultimate desire to move from VOs!)

web-app/src/app/models/folder-vo.ts

Lines 77 to 78 in daa410e

public thumbnail256: string;

public thumbnail256CloudPath: string;

cecilia-donnelly · 2025-09-19T19:07:17Z

src/app/shared/services/api/folder.repo.ts

+		status: stelaFolder.status,
+		publicDT: stelaFolder.publicAt,
+		parentFolderId: stelaFolder.parentFolder.id,
+		pathAsText: stelaFolder.paths.names,


(note to self) Plural?

src/app/shared/services/api/record.repo.ts

This continues our migration away from the PHP api / into stela. Similar to the work around get record we want to decouple the actual API calls from our component implementations. Once we're fully onto Stela we can update our components to use the stela data format.

cecilia-donnelly

So, yes, this is only used in two ways: one is to load subfolders of "Apps" (I only see these in accounts that I used for testing our Etherpad integration, which I don't have locally or in dev). Even if we break that, I'm not too worried about it. The basic left menu I can confirm is loading.

The other place is to get folder thumbnails, which we only display in the public archive. This was working too, once I tried in a private window (to avoid using cached "broken" thumbnails). Looking good!

slifty force-pushed the PER-10107-use-folder-get-stela branch 2 times, most recently from 22c3d46 to 8bc9934 Compare September 15, 2025 20:22

slifty force-pushed the PER-10107-use-record-get-v2 branch 2 times, most recently from 67f1542 to 43c79d1 Compare September 15, 2025 20:24

cecilia-donnelly requested review from aasandei-vsp and cecilia-donnelly September 19, 2025 18:48

aasandei-vsp reviewed Sep 22, 2025

View reviewed changes

slifty force-pushed the PER-10107-use-folder-get-stela branch from 8bc9934 to d00b5bf Compare September 25, 2025 15:28

slifty force-pushed the PER-10107-use-record-get-v2 branch from 43c79d1 to d47ad15 Compare September 25, 2025 15:29

slifty force-pushed the PER-10107-use-folder-get-stela branch from d00b5bf to c43a07c Compare September 25, 2025 15:31

aasandei-vsp approved these changes Sep 26, 2025

View reviewed changes

aasandei-vsp mentioned this pull request Sep 29, 2025

[PER-10107] Make preview show for unlisted shares #727

Merged

cecilia-donnelly reviewed Sep 30, 2025

View reviewed changes

slifty force-pushed the PER-10107-use-record-get-v2 branch 2 times, most recently from d67a8f3 to 685be13 Compare September 30, 2025 18:17

slifty force-pushed the PER-10107-use-folder-get-stela branch from c43a07c to b8b7fb5 Compare September 30, 2025 18:45

Base automatically changed from PER-10107-use-record-get-v2 to main September 30, 2025 21:49

cecilia-donnelly requested a review from omnignorant October 3, 2025 17:52

slifty force-pushed the PER-10107-use-folder-get-stela branch from b8b7fb5 to 3c5811e Compare October 6, 2025 16:15

cecilia-donnelly approved these changes Oct 7, 2025

View reviewed changes

slifty merged commit 1781403 into main Oct 7, 2025
6 of 8 checks passed

slifty deleted the PER-10107-use-folder-get-stela branch October 7, 2025 19:04

cecilia-donnelly mentioned this pull request Oct 7, 2025

PER-10307: Show thumbnails correctly for public folders #751

Merged

	public thumbnail256: string;
	public thumbnail256CloudPath: string;

Update getWithChildren to use stela #720

Update getWithChildren to use stela #720

Uh oh!

Conversation

slifty commented Sep 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

aasandei-vsp left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

slifty commented Sep 29, 2025

Uh oh!

aasandei-vsp commented Sep 30, 2025

Uh oh!

cecilia-donnelly left a comment

Choose a reason for hiding this comment

Uh oh!

cecilia-donnelly Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

slifty Sep 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cecilia-donnelly Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

slifty Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

cecilia-donnelly Sep 19, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cecilia-donnelly left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Update `getWithChildren` to use stela #720

Update `getWithChildren` to use stela #720

slifty commented Sep 15, 2025 •

edited

Loading

codecov bot commented Sep 25, 2025 •

edited

Loading

aasandei-vsp left a comment •

edited

Loading

slifty Sep 30, 2025 •

edited

Loading