Per 9589 milestone sort by cecilia-donnelly · Pull Request #958 · PermanentOrg/web-app

cecilia-donnelly · 2026-03-06T22:32:46Z

Allow users to display milestones on the public profile either chronologically or reverse chronologically.

I could use some help with making it obvious to the user (1) that this is a button and (2) what the button does. I don't know how to make sure that they understand that they are reordering the milestones on a different screen than the one they are looking at.

I have changed this in response to feedback to let users change the order in archive settings. I also point to that setting from the profile editor where members create milestones.

codecov · 2026-03-06T22:34:43Z

Codecov Report

❌ Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 49.64%. Comparing base (d86488f) to head (5ab3146).

Files with missing lines	Patch %	Lines
.../components/profile-edit/profile-edit.component.ts	0.00%	1 Missing ⚠️
src/app/shared/services/api/archive.repo.ts	0.00%	1 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #958   +/-   ##
=======================================
  Coverage   49.63%   49.64%           
=======================================
  Files         352      353    +1     
  Lines       11504    11530   +26     
  Branches     1953     1958    +5     
=======================================
+ Hits         5710     5724   +14     
- Misses       5600     5615   +15     
+ Partials      194      191    -3

☔ 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.

cecilia-donnelly · 2026-03-10T19:37:22Z

Loom for @omnignorant or anyone else: https://www.loom.com/share/c8d1f8f1cf6643c4a49f02e164263750

I should have turned off audio, but you can mute it - there's no narration. Or you can hear me clicking around if you put the sound on. :)

Copilot

Pull request overview

Adds support for choosing whether milestones on a public profile are displayed chronologically (oldest-first) or reverse-chronologically (newest-first), with a toggle exposed in the profile edit UI and persisted to the archive via a V2 PATCH.

Changes:

Add milestoneSortOrder to ArchiveVO and use it to drive milestone sorting on the public profile.
Add a profile-edit toggle button + saving state that PATCHes the archive setting.
Add unit tests covering both the public sorting behavior and the toggle behavior.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`src/app/shared/services/api/archive.repo.ts`	Adds `patchArchive()` to persist `milestoneSortOrder` via `PATCH v2/archive/:id`.
`src/app/public/components/public-profile/public-profile.component.ts`	Sorts milestones asc/desc based on `archive.milestoneSortOrder`.
`src/app/public/components/public-profile/public-profile.component.spec.ts`	New tests verifying default/newest-first vs chronological/oldest-first sorting.
`src/app/models/archive-vo.ts`	Adds `milestoneSortOrder` field to the archive model.
`src/app/core/components/profile-edit/profile-edit.component.ts`	Implements `toggleMilestoneOrder()` and saving guard flag.
`src/app/core/components/profile-edit/profile-edit.component.spec.ts`	Adds unit tests for toggle behavior and saving flag.
`src/app/core/components/profile-edit/profile-edit.component.html`	Adds reorder button to Milestones section and updates help text.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

aasandei-vsp

Just a small comment.

aasandei-vsp · 2026-03-11T15:17:50Z

+		const newOrder =
+			this.archive.milestoneSortOrder === 'chronological'
+				? 'reverse_chronological'
+				: 'chronological';


chronological and reverse_chronological seem to repeat in multiple places. I would create an enum or at least a type for them.

cecilia-donnelly · 2026-03-12T21:32:47Z

Holding off on this for some design help from @omnignorant.

omnignorant · 2026-03-13T16:42:19Z

Existing text in the profile editor under the Milestones heading:

Milestones capture any significant event in the history of this archive, and are ordered from most recent to least recent.
Change to: Milestones capture any significant event in the history this archive represents. Milestones are not sorted below, they appear in the order they are created. Milestones can be set to appear in ascending or descending time order on the Public Gallery page in the Archive Settings.
link to archive settings modal if possible.

Design guidance for milestone sort toggle.

as noted in the text above, put this in archive settings > Public Gallery Page Settings, below the type option;
mimic the show download button radio button format;
Bold text (title): Milestone sort order;
Radio button labels: descending | ascending;
help text: Select whether the most recent (descending) or most distant (ascending) milestones at the top of the list.

cecilia-donnelly · 2026-03-17T17:38:29Z

Converting this to a draft while I make the changes @omnignorant requested.

cecilia-donnelly · 2026-04-09T20:33:23Z

 	}

+	openArchiveSettings() {
+		this.dialog.open(ArchiveSettingsDialogComponent, { width: '1000px' });


@aasandei-vsp we are a little inconsistent about how we open modals - obviously a set width here is not responsive. Do you have a suggestion about the right way to do this?

This is definitely not ideal, but from what I see in the code, this is the default, hardcoding it to 1000px. Also, looking at the modal behavior, it does not seem to have a fixed 1000px width, as it is responsive and shrinks/widens with the screen. I have removed the config and the modal seems to behave the same. I'd recommend removing the width property completely and check the modal behavior. If there is anything not working correctly, than at most I would add a min-width of whatever you think should be, taking into account that are many screen sizes, but I doubt that would be necessary.(trial and error)

Add a radio button in archive settings that allows users to change the order in which milestones appear on their public profile.

aasandei-vsp · 2026-04-17T11:24:08Z

 	}

+	openArchiveSettings() {
+		this.dialog.open(ArchiveSettingsDialogComponent, { width: '1000px' });


This is definitely not ideal, but from what I see in the code, this is the default, hardcoding it to 1000px. Also, looking at the modal behavior, it does not seem to have a fixed 1000px width, as it is responsive and shrinks/widens with the screen. I have removed the config and the modal seems to behave the same. I'd recommend removing the width property completely and check the modal behavior. If there is anything not working correctly, than at most I would add a min-width of whatever you think should be, taking into account that are many screen sizes, but I doubt that would be necessary.(trial and error)

aasandei-vsp · 2026-04-17T11:28:01Z

+				this.milestoneSortOrder,
+			);
+		} catch (err) {
+			this.msg.showError({ message: err.error?.message, translate: true });


Have a look with the inspect at the error it should show. In my case, for patch record/folder, it was err.error.error. The err.error.message might be the more generic one related to response status.

aasandei-vsp · 2026-04-17T11:33:20Z


+	public async patchArchive(
+		archiveId: string,
+		milestoneSortOrder: 'chronological' | 'reverse_chronological',


This type appears in multiple places. It would be better to just put it in one place, maybe in the archive model. Something like type MilestoneSortOrder = 'chronological' | 'reverse_chronological'.

slifty · 2026-04-20T19:43:00Z

 	}

+	public async onMilestoneSortOrderChange() {
+		this.archive.milestoneSortOrder = this.milestoneSortOrder;


This assignment changes before the patch call is made, which means if the patch call fails we now have the client / server disagreeing.

I think the logic there is we want the state to update right away (does it result in a visual change?) rather than waiting on a network call to resolve. If that's the case, can you store the "old" value before making this assignment, and revert to that old value in the catch block?

If that isn't the case, can we move this down to the try block / after the patch call?

slifty · 2026-04-20T19:44:21Z

+				this.milestoneSortOrder,
+			);
+		} catch (err) {
+			this.msg.showError({ message: err.error?.message, translate: true });


Related to what @aasandei-vsp notes -- since this message is server generated we won't have a matching i8n string in the front end. Instead of rendering a server value can we detect the issue and render a web-app-defined value (with translation strings?)

slifty · 2026-04-20T19:47:12Z

 		).toPromise();
 	}

+	public async patchArchive(


This is generically named patchArchive but it takes a single required milestoneSortOrder

Should we name this updateMilestoneSortOrder?

Or, if it's intended to be a true patch method, can we define the parameters in a more "patchy" way (e.g. it takes a patchValues: Partial<ArchiveVO>, or even a constrained patchValues: Partial<Pick<ArchiveVO, 'milestoneSortOrder'>>)

slifty · 2026-04-20T19:48:09Z

-								archive, and are ordered from most recent to least recent.</em
+								archive, and by default are ordered from most recent to least
+								recent. This order can be changed in
+								<a href="javascript:void(0)" (click)="openArchiveSettings()"


I'm told by robots that "inline-js href is an a11y smell"

A <button class="link-styled"> (or anchor with
role="button" + (keydown.enter)) preserves keyboard + screen-reader semantics.

slifty · 2026-04-20T19:49:22Z

+		await TestBed.configureTestingModule({
+			declarations: [PublicProfileComponent],
+			providers: [PublicProfileService],
+			schemas: [NO_ERRORS_SCHEMA],


This is also a claude-identified issue:

NO_ERRORS_SCHEMA — this silences all unknown-element/attribute errors in the template, which can hide real regressions. Prefer importing the needed child components or using CUSTOM_ELEMENTS_SCHEMA only where necessary.

slifty · 2026-04-20T19:51:45Z

+
+			let updatingWhileSaving: boolean;
+			const originalPatchArchive = mockApiService.archive.patchArchive;
+			mockApiService.archive.patchArchive = async (id, order) => {


There's a weird race condition here! This modifies a shared mock. It is restored later (line 223), but if there is ever an exception in the test then we'd never get to line 223, and so future tests will use this modified mock.

We could use a try / finally to cover that case, we could make the mock itself more robust and add a flag, or we could spy on the mock (e.g. spyOn(mockApiService.archive, 'patchArchive').and.callFake(...))

cecilia-donnelly force-pushed the per-9589-milestone-sort branch 5 times, most recently from 755b2ff to 0f764af Compare March 10, 2026 18:57

cecilia-donnelly marked this pull request as ready for review March 10, 2026 19:17

cecilia-donnelly requested review from aasandei-vsp and slifty March 10, 2026 19:18

slifty requested a review from Copilot March 10, 2026 19:39

Copilot started reviewing on behalf of slifty March 10, 2026 19:40 View session

Copilot AI reviewed Mar 10, 2026

View reviewed changes

Comment thread src/app/core/components/profile-edit/profile-edit.component.spec.ts Outdated

Comment thread src/app/models/archive-vo.ts

Comment thread src/app/shared/services/api/archive.repo.ts Outdated

Comment thread src/app/core/components/profile-edit/profile-edit.component.ts Outdated

aasandei-vsp reviewed Mar 11, 2026

View reviewed changes

cecilia-donnelly marked this pull request as draft March 17, 2026 17:38

cecilia-donnelly force-pushed the per-9589-milestone-sort branch from 4828657 to 0b4dec5 Compare March 30, 2026 15:38

cecilia-donnelly force-pushed the per-9589-milestone-sort branch 3 times, most recently from 6c06841 to 48e182b Compare April 9, 2026 20:31

cecilia-donnelly commented Apr 9, 2026

View reviewed changes

cecilia-donnelly marked this pull request as ready for review April 9, 2026 20:37

cecilia-donnelly requested a review from aasandei-vsp April 9, 2026 20:37

PER-9589: Toggle milestone sort order

5ab3146

Add a radio button in archive settings that allows users to change the order in which milestones appear on their public profile.

cecilia-donnelly force-pushed the per-9589-milestone-sort branch from 48e182b to 5ab3146 Compare April 16, 2026 18:01

aasandei-vsp reviewed Apr 17, 2026

View reviewed changes

slifty requested changes Apr 20, 2026

View reviewed changes

Conversation

cecilia-donnelly commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Mar 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

cecilia-donnelly commented Mar 10, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

aasandei-vsp left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cecilia-donnelly commented Mar 12, 2026

Uh oh!

omnignorant commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cecilia-donnelly commented Mar 17, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

cecilia-donnelly commented Mar 6, 2026 •

edited

Loading

codecov Bot commented Mar 6, 2026 •

edited

Loading

omnignorant commented Mar 13, 2026 •

edited

Loading