diff --git a/docs/openapi.yaml b/docs/openapi.yaml index ca7cd6b..1f1d92e 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -3126,10 +3126,20 @@ paths: candidates: type: array items: { $ref: "#/components/schemas/Person" } + follows: + description: > + The notes this one points at with [[links]], which are shared + alongside it. Owner's own notes only. + type: array + items: { $ref: "#/components/schemas/NoteFollow" } "404": { $ref: "#/components/responses/NotFound" } post: tags: [notes] summary: Share a note with a person, or change the role they hold. + description: > + The notes this one points at with [[links]] are shared with the same person + and the same role, so a link in a shared note leads somewhere. Only notes + the sharer owns follow, and the response lists them. requestBody: required: true content: @@ -3141,7 +3151,16 @@ paths: user_id: { type: string } role: { type: string, enum: [viewer, editor] } responses: - "204": { description: Shared } + "200": + description: Shared + content: + application/json: + schema: + type: object + properties: + follows: + type: array + items: { $ref: "#/components/schemas/NoteFollow" } "404": { $ref: "#/components/responses/NotFound" } "422": { $ref: "#/components/responses/Validation" } @@ -4035,6 +4054,23 @@ components: properties: user: { $ref: "#/components/schemas/Person" } role: { type: string, enum: [viewer, editor] } + linked: + description: > + True when the person has this note because another shared note points + at it, rather than because it was shared with them on its own. Such a + grant is taken back by removing the link or the share it came from, not + by removing it here. + type: boolean + via: + description: The title of the shared note that points at this one. + type: string + + NoteFollow: + type: object + description: A note that is shared alongside another because that one links to it. + properties: + note_id: { type: string } + title: { type: string } Mailbox: type: object diff --git a/internal/httpapi/notes_handlers.go b/internal/httpapi/notes_handlers.go index 3c0e3d2..47b2045 100644 --- a/internal/httpapi/notes_handlers.go +++ b/internal/httpapi/notes_handlers.go @@ -204,9 +204,49 @@ func (s *Server) handleCreateNote(w http.ResponseWriter, r *http.Request) { s.internal(w, r, "create note", err) return } + // En ny note kan være den, en delt note hele tiden har peget på: linket er + // skrevet som en titel, og titlen fandtes ikke før nu. + // + // Kun når der står noget i den. En tom note har ingen titel, og et link peger på + // en titel — så der er intet at regne, og det kostede alligevel: udregningen + // åbner hver eneste af ejerens noter, og den lå her mellem "Ny note" og det + // svar, fladen venter på for at kunne tegne editoren. Halvfems millisekunder er + // nok til at nå at taste den første linje ind i den forrige note, som så blev + // tegnet væk. Målt i e2e-prøven for monospace, der begyndte at tabe sin titel. + if strings.TrimSpace(n.Body) != "" { + s.followLinks(r, n) + } writeJSON(w, http.StatusCreated, n) } +// followLinks holder de udledte delinger i takt med teksten. +// +// De noter, en delt note peger på, følger med — og teksten kan ændre sig, efter +// delingen er sat. Et link skrevet i dag i en note, der blev delt i går, skal føre +// samme sted hen for begge. +// +// Kun ejerens egne gemninger regner om, og det er ikke en optimering. Et link +// peger på en titel, og en medredaktør må skrive i teksten — så hvis enhver +// gemning talte med, kunne den, man har delt en note med, skrive +// `[[Bankoplysninger]]` ind i den og dermed dele den med sig selv. +// +// Det lukker ikke hullet helt, og det skal siges frem for at lyde lukket: skriver +// medredaktøren linket, og gemmer ejeren bagefter sin egen note, følger den note +// med dér. Det, der står tilbage mod det, er synlighed — linket står som tekst i +// ejerens egen note, og delingspanelet viser hver note, der er fulgt med, og hvem +// der har den. En stille eskalering er det ikke; en, man kan overse, er den. +// +// Fejler det, siges det i loggen og ikke til den, der gemte: noten *er* gemt, og +// et svar, der lyder som om den ikke er, ville få dem til at skrive den igen. +func (s *Server) followLinks(r *http.Request, n *store.Note) { + if n == nil || n.CreatedBy != userFrom(r.Context()).ID { + return + } + if _, err := s.db.SyncLinkedShares(r.Context(), n.CreatedBy); err != nil { + s.log.Error("sync linked shares", "err", err, "note", n.ID) + } +} + func (s *Server) handleUpdateNote(w http.ResponseWriter, r *http.Request) { var body noteBody if err := decodeJSON(w, r, &body); err != nil { @@ -242,6 +282,7 @@ func (s *Server) handleUpdateNote(w http.ResponseWriter, r *http.Request) { } n.ProjectID = *body.ProjectID } + before := n.Body if body.Body != nil { n.Body = *body.Body } @@ -249,13 +290,41 @@ func (s *Server) handleUpdateNote(w http.ResponseWriter, r *http.Request) { n.Pinned = *body.Pinned } + // Læst før gemningen, sammenlignet efter. En gemning er en tastepause, ikke en + // begivenhed, så de fleste af dem ændrer intet af det her — og udregningen + // åbner hver eneste af ejerens noter, fordi titlerne er forseglet. Den skal + // køre, når svaret kan være blevet et andet, og ellers ikke. + wasLinking := store.LinksIn(before) + wasTitled := n.Title + if err := s.db.SaveNote(r.Context(), n); err != nil { s.internal(w, r, "update note", err) return } + // Titlen tæller med: den *er* første linje, så en omskrevet overskrift flytter + // det, andres links peger på, uden at et eneste link er rørt. + if n.Title != wasTitled || !sameLinks(wasLinking, store.LinksIn(n.Body)) { + s.followLinks(r, n) + } writeJSON(w, http.StatusOK, n) } +func sameLinks(a, b []store.NoteLink) bool { + if len(a) != len(b) { + return false + } + in := make(map[store.NoteLink]bool, len(a)) + for _, l := range a { + in[l] = true + } + for _, l := range b { + if !in[l] { + return false + } + } + return true +} + func (s *Server) handleDeleteNote(w http.ResponseWriter, r *http.Request) { n, err := s.db.Note(r.Context(), chi.URLParam(r, "noteID")) if err != nil { @@ -279,6 +348,9 @@ func (s *Server) handleDeleteNote(w http.ResponseWriter, r *http.Request) { s.internal(w, r, "delete note", err) return } + // En note i papirkurven er ikke længere et udgangspunkt for noget. Det, den tog + // med sig, skal derfor heller ikke blive stående hos dem, den blev delt med. + s.followLinks(r, n) w.WriteHeader(http.StatusNoContent) } diff --git a/internal/httpapi/notes_share_handlers.go b/internal/httpapi/notes_share_handlers.go index 6796639..8a7864a 100644 --- a/internal/httpapi/notes_share_handlers.go +++ b/internal/httpapi/notes_share_handlers.go @@ -77,12 +77,37 @@ func (s *Server) handleListNoteShares(w http.ResponseWriter, r *http.Request) { s.internal(w, r, "list note shares", err) return } + // Regnet forfra, én gang, og læst to veje ud af det samme svar: hvem der har + // *denne* note, fordi en anden peger på den, og hvilke noter der omvendt følger + // med den her. To udregninger af det samme er to, der kan blive uenige. + // + // Og forfra frem for læst af basen, så panelet viser det, der gælder nu — også + // når linkene er skrevet af en anden og delingerne derfor ikke er regnet om + // siden. + following, err := s.db.SyncLinkedShares(r.Context(), userFrom(r.Context()).ID) + if err != nil { + s.internal(w, r, "sync linked shares", err) + return + } + via := map[string]string{} + for _, l := range following { + if l.NoteID == n.ID { + via[l.UserID] = l.Via + } + } + out := make([]noteShareJSON, 0, len(shares)) already := map[string]bool{} for _, sh := range shares { + // Panelet skal kunne sige forskel: en deling, nogen har valgt, tages tilbage + // ved at fjerne den — en, der er fulgt med, ved at fjerne linket eller den + // deling, den kom fra. En knap, der lover det første og gør det andet, er + // værre end ingen knap. out = append(out, noteShareJSON{ - User: personJSON{ID: sh.User.ID, Name: sh.User.Name, AvatarColor: sh.User.AvatarColor}, - Role: string(sh.Role), + User: personJSON{ID: sh.User.ID, Name: sh.User.Name, AvatarColor: sh.User.AvatarColor}, + Role: string(sh.Role), + Linked: via[sh.User.ID] != "", + Via: via[sh.User.ID], }) already[sh.User.ID] = true } @@ -103,12 +128,41 @@ func (s *Server) handleListNoteShares(w http.ResponseWriter, r *http.Request) { cand = append(cand, personJSON{ID: p.ID, Name: p.Name, AvatarColor: p.AvatarColor}) } - writeJSON(w, http.StatusOK, map[string]any{"shares": out, "candidates": cand}) + writeJSON(w, http.StatusOK, map[string]any{ + "shares": out, + "candidates": cand, + "follows": followsOf(following, n.ID), + }) +} + +// followsOf er de noter, der følger med *denne* note — én linje pr. note, ikke én +// pr. note og person. Panelet spørger på vegne af noten, og svaret "Prisliste +// følger med" er det samme, hvem det så end deles med. +func followsOf(all []store.LinkedShare, noteID string) []followJSON { + seen := map[string]bool{} + out := []followJSON{} + for _, l := range all { + if l.ViaID != noteID || seen[l.NoteID] { + continue + } + seen[l.NoteID] = true + out = append(out, followJSON{NoteID: l.NoteID, Title: l.Title}) + } + return out +} + +type followJSON struct { + NoteID string `json:"note_id"` + Title string `json:"title"` } type noteShareJSON struct { User personJSON `json:"user"` Role string `json:"role"` + // Sat, når personen har noten, fordi en anden delt note peger på den — ikke + // fordi nogen har delt netop den. `via` er den note, der peger. + Linked bool `json:"linked"` + Via string `json:"via,omitempty"` } type shareNoteRequest struct { @@ -159,7 +213,15 @@ func (s *Server) handleShareNote(w http.ResponseWriter, r *http.Request) { s.storeError(w, r, "share note", err) return } - w.WriteHeader(http.StatusNoContent) + + // Og det, noten peger på, følger med. Svaret siger hvad — en deling, der stille + // tog tre noter mere med, ville være rigtig og alligevel ikke til at overskue. + followed, err := s.db.SyncLinkedShares(r.Context(), userFrom(r.Context()).ID) + if err != nil { + s.internal(w, r, "sync linked shares", err) + return + } + writeJSON(w, http.StatusOK, map[string]any{"follows": followsOf(followed, n.ID)}) } // handleUnshareNote takes a person's access away again. @@ -177,5 +239,12 @@ func (s *Server) handleUnshareNote(w http.ResponseWriter, r *http.Request) { s.internal(w, r, "unshare note", err) return } + // Det, der fulgte med, følger også med tilbage. Regnet forfra: de udledte rækker + // skrives om fra bunden, så en note, der stadig kan nås fra en anden deling, + // bliver stående, og en, der ikke kan, forsvinder. + if _, err := s.db.SyncLinkedShares(r.Context(), userFrom(r.Context()).ID); err != nil { + s.internal(w, r, "sync linked shares", err) + return + } w.WriteHeader(http.StatusNoContent) } diff --git a/internal/httpapi/notes_share_test.go b/internal/httpapi/notes_share_test.go index 9abff0b..d9a5c5f 100644 --- a/internal/httpapi/notes_share_test.go +++ b/internal/httpapi/notes_share_test.go @@ -46,7 +46,7 @@ func TestSharedNoteReachesTheOtherPerson(t *testing.T) { // Share it as a viewer. resp, _ := owner.do(t, "POST", "/api/v1/notes/"+noteID+"/shares", map[string]string{"user_id": sofie, "role": "viewer"}) - if resp.StatusCode != http.StatusNoContent { + if resp.StatusCode != http.StatusOK { t.Fatalf("share: status %d", resp.StatusCode) } @@ -92,7 +92,7 @@ func TestSharedEditorCanWriteButNotGiveAway(t *testing.T) { noteID := owner.createNote(t, "# Delt\noprindelig") if resp, _ := owner.do(t, "POST", "/api/v1/notes/"+noteID+"/shares", - map[string]string{"user_id": sofie, "role": "editor"}); resp.StatusCode != http.StatusNoContent { + map[string]string{"user_id": sofie, "role": "editor"}); resp.StatusCode != http.StatusOK { t.Fatalf("share as editor failed") } @@ -193,7 +193,104 @@ func TestShareRejectsUnknownAndSelf(t *testing.T) { t.Errorf("sharing with oneself: %d, want 422", resp.StatusCode) } if resp, _ := owner.do(t, "POST", "/api/v1/notes/"+noteID+"/shares", - map[string]string{"user_id": me, "role": "owner"}); resp.StatusCode == http.StatusNoContent { + map[string]string{"user_id": me, "role": "owner"}); resp.StatusCode == http.StatusOK { t.Error("a note was shared at the owner role") } } + +// De noter, en delt note peger på, følger med — og holder op igen. +// +// Det var meldt som en fejl den anden vej rundt: linkene i en delt note førte +// ingen steder hen, fordi det, de pegede på, ikke var delt. Prøven går hele vejen +// rundt om svaret på det — dybden, teksten der ændrer sig bagefter, det der ikke +// må følge med, og vejen tilbage. +func TestSharedNoteTakesItsLinksWithIt(t *testing.T) { + owner := newTestServer(t) + owner.bootstrap(t) + other := owner.newUser(t, "sofie@example.dk", "Sofie") + sofie := userID(t, owner, "sofie@example.dk") + + // Prisliste peger videre på Leveringstider, så kæden er to led lang. + deep := owner.createNote(t, "# Leveringstider\nfire dage") + linked := owner.createNote(t, "# Prisliste\nog [[Leveringstider]]") + apart := owner.createNote(t, "# Bankoplysninger\nikke til deling") + noteID := owner.createNote(t, "# Aftale om levering\nse [[Prisliste]]") + + reachable := func(id string) bool { + resp, _ := other.do(t, "GET", "/api/v1/notes/"+id, nil) + return resp.StatusCode == http.StatusOK + } + + resp, out := owner.do(t, "POST", "/api/v1/notes/"+noteID+"/shares", + map[string]string{"user_id": sofie, "role": "viewer"}) + if resp.StatusCode != http.StatusOK { + t.Fatalf("share: status %d", resp.StatusCode) + } + // Svaret siger, hvad der fulgte med — hele kæden, ikke kun første led. En + // deling, der stille tog to noter mere med, ville være rigtig og alligevel ikke + // til at overskue. + follows, _ := out["follows"].([]any) + if len(follows) != 2 { + t.Fatalf("delingen nævnte %d noter, der fulgte med, want 2: %v", len(follows), out["follows"]) + } + + if !reachable(linked) { + t.Error("noten, den delte note peger på, fulgte ikke med") + } + if !reachable(deep) { + t.Error("kæden stoppede efter ét led") + } + if reachable(apart) { + t.Error("en note, ingen peger på, blev delt") + } + + // Skrevet bagefter tæller også. Et link i dag i en note delt i går skal føre + // samme sted hen for begge. + senere := owner.createNote(t, "# Rabatter\nti procent") + if reachable(senere) { + t.Fatal("en helt ny note var delt, før noget pegede på den") + } + if resp, _ := owner.do(t, "PATCH", "/api/v1/notes/"+noteID, + map[string]any{"body": "# Aftale om levering\nse [[Prisliste]] og [[Rabatter]]"}); resp.StatusCode != http.StatusOK { + t.Fatal("kunne ikke skrive videre i den delte note") + } + if !reachable(senere) { + t.Error("et link skrevet efter delingen fulgte ikke med") + } + + // Og vejen tilbage: tages delingen tilbage, følger det med, den tog med sig. + if resp, _ := owner.do(t, "DELETE", "/api/v1/notes/"+noteID+"/shares/"+sofie, nil); resp.StatusCode != http.StatusNoContent { + t.Fatalf("unshare: status %d", resp.StatusCode) + } + if reachable(linked) || reachable(deep) || reachable(senere) { + t.Error("det, der fulgte med delingen, blev stående, da den blev taget tilbage") + } +} + +// En medredaktør kan ikke dele ejerens noter med sig selv ved at skrive et link. +// +// Det er den ene vej, "linkene følger med" kunne blive til en rettighedsfejl: et +// link peger på en titel, og enhver, der må rette i en delt note, kan skrive en +// titel. Derfor regnes delingerne kun om, når ejeren selv har gemt. +func TestAnEditorCannotShareTheOwnersNotesWithThemselves(t *testing.T) { + owner := newTestServer(t) + owner.bootstrap(t) + other := owner.newUser(t, "sofie@example.dk", "Sofie") + sofie := userID(t, owner, "sofie@example.dk") + + secret := owner.createNote(t, "# Bankoplysninger\nreg og konto") + noteID := owner.createNote(t, "# Fælles\nintet link her") + if resp, _ := owner.do(t, "POST", "/api/v1/notes/"+noteID+"/shares", + map[string]string{"user_id": sofie, "role": "editor"}); resp.StatusCode != http.StatusOK { + t.Fatal("share as editor failed") + } + + // Hun skriver linket selv, i den note hun har fået lov at rette i. + if resp, _ := other.do(t, "PATCH", "/api/v1/notes/"+noteID, + map[string]any{"body": "# Fælles\nse [[Bankoplysninger]]"}); resp.StatusCode != http.StatusOK { + t.Fatal("medredaktøren kunne ikke skrive") + } + if resp, _ := other.do(t, "GET", "/api/v1/notes/"+secret, nil); resp.StatusCode == http.StatusOK { + t.Error("en medredaktør delte ejerens note med sig selv ved at skrive et link") + } +} diff --git a/internal/store/migrations/0032_shares_follow_links.sql b/internal/store/migrations/0032_shares_follow_links.sql new file mode 100644 index 0000000..906eb39 --- /dev/null +++ b/internal/store/migrations/0032_shares_follow_links.sql @@ -0,0 +1,22 @@ +-- De noter, en delt note peger på, følger med delingen. +-- +-- Et `[[link]]` i en note, der er delt, pegede indtil nu på noget, modtageren +-- ikke havde fået. Linket stod der, kunne ses, og førte ingen steder hen — meldt +-- af den første, der prøvede at læse en delt note, og med rette: den, der deler +-- en note, deler det, noten *siger*, og et link er en del af det, den siger. +-- +-- Rækken er den samme som en almindelig deling. `linked` er den ene forskel: den +-- siger, at rækken ikke er valgt af nogen, men udledt af, hvad noten peger på. +-- Det gør to ting muligt, som en boolsk værdi ellers sjældent gør sig fortjent +-- til. Fladen kan sige *hvorfor* en person har den note — "den fulgte med" er en +-- anden sætning end "du delte den". Og udledningen kan regnes forfra hver gang: +-- de udledte rækker må slettes og skrives igen efter hver ændring, mens de +-- valgte aldrig røres. Uden skellet ville en oprydning enten lade forældede +-- delinger blive stående eller fjerne dem, ejeren selv havde sat. +-- +-- Kun ejerens egne noter følger med. Et link peger på en *titel*, og titler kan +-- skrives af enhver, der må rette i noten — så hvis et link kunne hente hvad som +-- helst, ville en medredaktør kunne skrive `[[Bankoplysninger]]` og læse med. +-- Opslaget sker derfor kun blandt de noter, ejeren selv ejer, og deling af dem +-- har ejeren allerede ret til. +ALTER TABLE note_shares ADD COLUMN linked INTEGER NOT NULL DEFAULT 0; diff --git a/internal/store/note_shares.go b/internal/store/note_shares.go index c8e6468..c027d99 100644 --- a/internal/store/note_shares.go +++ b/internal/store/note_shares.go @@ -4,6 +4,8 @@ import ( "context" "database/sql" "errors" + "sort" + "strings" "time" ) @@ -131,3 +133,242 @@ func (db *DB) NotesSharedWith(ctx context.Context, userID string, noteIDs []stri } return out, rows.Err() } + +// --- de noter, en delt note peger på --------------------------------------------- + +// LinkedShare is one note that followed a share: which note, what it is called, +// and which of the owner's shared notes brought it. The panel shows all three — +// "Prisliste came along with Aftale om levering" is a sentence somebody can act +// on, where a bare id is not. +type LinkedShare struct { + NoteID string `json:"note_id"` + Title string `json:"title"` + ViaID string `json:"via_id"` + Via string `json:"via"` + Role Role `json:"role"` + UserID string `json:"user_id"` +} + +// Hvor langt en deling må brede sig. +// +// En note, der peger på en note, der peger på en note, er stadig én ting at læse, +// og den skal følge med. Men noter kan pege i ring og i vifte, og "del denne ene +// note" må ikke kunne blive til fire hundrede. Loftet er ikke en optimering: det +// er den grænse, hvor en deling holder op med at være noget, ejeren kan overskue. +const linkedShareLimit = 100 + +// SyncLinkedShares regner de udledte delinger forfra for alt, hvad én person ejer. +// +// Kaldt efter enhver af ejerens egne handlinger, der kan flytte svaret: en deling, +// en fjernet deling, og en gemning, der har ændret på det, noten peger på. Regnet +// forfra frem for rettet til, fordi det er den samme kode hver gang og derfor det +// samme resultat hver gang — en tilføjelse og en fjernelse er ikke to veje gennem +// koden, men det samme kald med et andet udgangspunkt. +// +// Kun *direkte* delinger er udgangspunkt. En note i et delt projekt er delt af +// projektet, og projektets folk kan se projektets noter; det er en anden vej ind, +// med sine egne regler, og den blander sig ikke her. +// +// Returnerer, hvad der nu følger med, så kalderen kan sige det. Det er meningen at +// det bliver sagt: en deling, der stille tager tre noter mere med, er ikke til at +// overskue, uanset hvor rigtigt den gør det. +func (db *DB) SyncLinkedShares(ctx context.Context, ownerID string) ([]LinkedShare, error) { + if ownerID == "" { + return nil, nil + } + + // Har personen ikke delt noget, er der intet at regne — og det er de fleste, + // det meste af tiden. Uden det her ville hvert eneste klik i notelisten åbne + // hver eneste note for at finde ud af, at svaret er tomt: fladen spørger til + // delingerne, hver gang man vælger en note, man selv ejer. + // + // Ét spørgsmål dækker begge halvdele. Er der ingen rækker overhovedet, er der + // hverken en deling at brede ud eller en udledt række at rydde op efter. + var anyShare int + if err := db.QueryRowContext(ctx, ` + SELECT EXISTS( + SELECT 1 FROM note_shares s + JOIN notes n ON n.id = s.note_id + WHERE n.created_by = ?)`, ownerID).Scan(&anyShare); err != nil { + return nil, err + } + if anyShare == 0 { + return nil, nil + } + + // Ejerens egne noter, åbnet. Titlerne er forseglet i basen, og et link peger på + // en titel — så der er ikke noget opslag at lave i SQL. Målt andetsteds i denne + // fil: tolv hundrede noter koster nogle og tredive millisekunder at åbne, og det + // her kører kun, når ejeren selv har gjort noget. + mine, err := db.notesWhere(ctx, `WHERE created_by = ? AND deleted_at IS NULL`, ownerID) + if err != nil { + return nil, err + } + byTitle := map[string]string{} + title := map[string]string{} + for _, n := range mine { + title[n.ID] = n.Title + key := strings.ToLower(strings.TrimSpace(n.Title)) + // Den første vinder, og listen kommer sorteret med den senest rørte først: + // to noter med samme titel er sjældent, og når det sker, er den, man skrev + // sidst, den, man mente. + if key != "" { + if _, taken := byTitle[key]; !taken { + byTitle[key] = n.ID + } + } + } + + // Hvad hver note peger på, i ét spørgsmål frem for ét pr. note. + links := map[string][]string{} + rows, err := db.QueryContext(ctx, ` + SELECT l.note_id, l.target_id + FROM note_links l + JOIN notes n ON n.id = l.note_id + WHERE l.kind = 'note' AND n.created_by = ? AND n.deleted_at IS NULL`, ownerID) + if err != nil { + return nil, err + } + for rows.Next() { + var from, target string + if err := rows.Scan(&from, &target); err != nil { + rows.Close() + return nil, err + } + links[from] = append(links[from], target) + } + rows.Close() + if err := rows.Err(); err != nil { + return nil, err + } + + // De delinger, ejeren selv har sat. + type grant struct{ noteID, userID string } + direct := map[grant]Role{} + rows, err = db.QueryContext(ctx, ` + SELECT s.note_id, s.user_id, s.role + FROM note_shares s + JOIN notes n ON n.id = s.note_id + WHERE s.linked = 0 AND n.created_by = ? AND n.deleted_at IS NULL`, ownerID) + if err != nil { + return nil, err + } + for rows.Next() { + var g grant + var role Role + if err := rows.Scan(&g.noteID, &g.userID, &role); err != nil { + rows.Close() + return nil, err + } + if role.Valid() { + direct[g] = role + } + } + rows.Close() + if err := rows.Err(); err != nil { + return nil, err + } + + // Vandringen. Pr. person, fordi det er personen, delingen handler om: to + // forskellige mennesker kan have to forskellige noter og derfor to forskellige + // vifter ud fra dem. + wanted := map[grant]LinkedShare{} + perUser := map[string][]grant{} + for g := range direct { + perUser[g.userID] = append(perUser[g.userID], g) + } + for userID, roots := range perUser { + seen := map[string]bool{} + type step struct { + noteID string + via string + } + + // Medredaktør først, læser bagefter, og hver runde tømt for sig. + // + // En note kan nås fra to delte noter med hver sin rolle, og så skal den + // stærkeste gælde. Én kø med det hele i ville afgøre det på afstand frem for + // på rolle: et barnebarn af en redigerbar note ville tabe til et barn af en + // læsbar, fordi det stod længere bagude i køen. To runder gør reglen til + // rækkefølgen — er noten allerede nået som redigerbar, ser den anden runde + // den ikke. + for _, role := range []Role{RoleEditor, RoleViewer} { + var queue []step + for _, g := range roots { + if direct[g] != role || seen[g.noteID] { + continue + } + seen[g.noteID] = true + queue = append(queue, step{noteID: g.noteID, via: g.noteID}) + } + for len(queue) > 0 && len(seen) <= linkedShareLimit { + cur := queue[0] + queue = queue[1:] + for _, target := range links[cur.noteID] { + id, ok := byTitle[strings.ToLower(strings.TrimSpace(target))] + // Et link, der ikke rammer en af ejerens egne noter, er enten + // dødt eller peger på en andens note. Ingen af delene er ejerens + // at dele. + if !ok || seen[id] { + continue + } + seen[id] = true + // Rollen arves fra den note, man kom fra. En medredaktør på en + // note bliver medredaktør på det, den peger på: teksten er én + // ting at arbejde i, og et link midt i den er ikke en grænse. + if _, isDirect := direct[grant{noteID: id, userID: userID}]; !isDirect { + wanted[grant{noteID: id, userID: userID}] = LinkedShare{ + NoteID: id, + Title: title[id], + ViaID: cur.via, + Via: title[cur.via], + Role: role, + UserID: userID, + } + } + queue = append(queue, step{noteID: id, via: cur.via}) + } + } + } + } + + // Skrevet i én transaktion: mellem sletningen og skrivningen står basen med + // færre delinger, end den skal, og det er ikke et vindue nogen skal kunne læse i. + err = db.Tx(ctx, func(tx *sql.Tx) error { + if _, err := tx.ExecContext(ctx, ` + DELETE FROM note_shares + WHERE linked = 1 + AND note_id IN (SELECT id FROM notes WHERE created_by = ?)`, ownerID); err != nil { + return err + } + for g, l := range wanted { + // DO NOTHING ved sammenstød: en direkte deling på den samme note er + // ejerens eget valg og har forrang. Den kan ikke nå herned — den er + // sorteret fra ovenfor — men reglen skal stå dér, hvor rækken skrives, + // ikke kun dér, hvor den udregnes. + if _, err := tx.ExecContext(ctx, ` + INSERT INTO note_shares (note_id, user_id, role, created_by, created_at, linked) + VALUES (?, ?, ?, ?, ?, 1) + ON CONFLICT (note_id, user_id) DO NOTHING`, + g.noteID, g.userID, l.Role, ownerID, time.Now().Unix()); err != nil { + return err + } + } + return nil + }) + if err != nil { + return nil, err + } + + out := make([]LinkedShare, 0, len(wanted)) + for _, l := range wanted { + out = append(out, l) + } + sort.Slice(out, func(i, j int) bool { + if out[i].UserID != out[j].UserID { + return out[i].UserID < out[j].UserID + } + return strings.ToLower(out[i].Title) < strings.ToLower(out[j].Title) + }) + return out, nil +} diff --git a/web/e2e/smoke.spec.js b/web/e2e/smoke.spec.js index a6dfb59..a44eadc 100644 --- a/web/e2e/smoke.spec.js +++ b/web/e2e/smoke.spec.js @@ -2875,13 +2875,13 @@ test('note links i en delt note fører til den rigtige note, eller siger hvorfor await sidebar.getByRole('link', { name: 'Noter', exact: true }).click(); await page.getByRole('button', { name: 'Ny note' }).click(); await page.getByRole('textbox', { name: 'Notens tekst' }).click(); - await page.keyboard.type('Prisliste for foraaret'); + await page.keyboard.type('Kristians prisblad'); await page.getByRole('textbox', { name: 'Notens tekst' }).blur(); await page.getByRole('button', { name: 'Ny note' }).click(); await page.getByRole('textbox', { name: 'Notens tekst' }).click(); - await page.keyboard.type('Aftale om levering\n'); - await page.keyboard.type('[[Prisliste for foraaret]]'); + await page.keyboard.type('Faelles aftale om afhentning\n'); + await page.keyboard.type('[[Kristians prisblad]]'); await page.getByRole('textbox', { name: 'Notens tekst' }).blur(); // Gæsten har en note af sin egen, som hun kan pege på. @@ -2891,7 +2891,7 @@ test('note links i en delt note fører til den rigtige note, eller siger hvorfor await gæst.keyboard.type('Linneas egne maal'); await gæst.getByRole('textbox', { name: 'Notens tekst' }).blur(); - await page.locator('.notes').getByRole('button', { name: /Aftale om levering/ }).click(); + await page.locator('.notes button.row').filter({ hasText: /^Faelles aftale/ }).click(); await page.locator('.sharewrap > button').click(); const share = page.locator('.sharepanel'); await expect(share).toBeVisible(); @@ -2901,19 +2901,19 @@ test('note links i en delt note fører til den rigtige note, eller siger hvorfor await expect(row).toBeVisible(); await row.locator('select').selectOption('editor'); - // Hun skriver sit eget link ind i den delte note. + // Hun skriver sit eget link ind i den delte note — og et, der ikke rammer noget. await gæst.goto(noterURL); - await gæst.locator('.notes').getByRole('button', { name: /Aftale om levering/ }).click(); + await gæst.locator('.notes button.row').filter({ hasText: /^Faelles aftale/ }).click(); const body = gæst.getByRole('textbox', { name: 'Notens tekst' }); - await expect(body).toContainText('Prisliste for foraaret'); + await expect(body).toContainText('Kristians prisblad'); await body.click(); await gæst.keyboard.press('Control+End'); - await gæst.keyboard.type('\n[[Linneas egne maal]]'); + await gæst.keyboard.type('\n[[Linneas egne maal]]\n[[Noget der aldrig blev skrevet]]'); await body.blur(); await expect(gæst.locator('footer .hint')).toHaveText('Gemt'); await gæst.goto(noterURL); - await gæst.locator('.notes').getByRole('button', { name: /Aftale om levering/ }).click(); + await gæst.locator('.notes button.row').filter({ hasText: /^Faelles aftale/ }).click(); const delt = gæst.getByRole('textbox', { name: 'Notens tekst' }); // Hendes eget link fører derhen, det siger. @@ -2922,22 +2922,118 @@ test('note links i en delt note fører til den rigtige note, eller siger hvorfor 'Linneas egne maal' ); - // Ejerens link peger på en note, hun ikke har fået. Den må ikke lande et - // tilfældigt sted — og den må sige hvorfor. - await gæst.locator('.notes').getByRole('button', { name: /Aftale om levering/ }).click(); + // Og det link, der ikke rammer nogen note, må ikke lande et tilfældigt sted. Det + // er dér, det gamle fald tilbage på "første søgeresultat" åbnede noten, man + // allerede stod i — hvilket ser ud som ingenting. + await gæst.locator('.notes button.row').filter({ hasText: /^Faelles aftale/ }).click(); const igen = gæst.getByRole('textbox', { name: 'Notens tekst' }); - await expect(igen).toContainText('Prisliste for foraaret'); - await igen.locator('a.notelink', { hasText: 'Prisliste for foraaret' }).click(); + await igen.locator('a.notelink', { hasText: 'Noget der aldrig blev skrevet' }).click(); await expect(gæst.locator('.toast')).toContainText('ikke delt med dig'); // Stadig i den delte note: linket flyttede hende ingen steder hen. await expect(gæst.getByRole('textbox', { name: 'Notens tekst' })).toContainText( - 'Aftale om levering' + 'Faelles aftale om afhentning' ); await gæstCtx.close(); expect(trouble).toEqual([]); }); +/** + * Delingen tager de noter med, noten peger på. + * + * Den anden halvdel af den melding: linkene i en delt note skal ikke bare sige + * hvorfor de ikke kan følges — de skal kunne følges. Deler man en note, deler man + * det, den *siger*, og et link er en del af det. + */ +test('en delt note tager de noter med, den peger på', async ({ browser, page }) => { + const trouble = watchForTrouble(page); + + await page.goto('/indstillinger/brugere'); + await page.getByLabel('E-mailadresse').fill('med@example.dk'); + await page.getByRole('button', { name: 'Send invitation' }).click(); + const link = await page.locator('.link-out').textContent(); + + const ctx = await browser.newContext({ storageState: { cookies: [], origins: [] } }); + const gæst = await ctx.newPage(); + await gæst.goto(link); + await gæst.getByLabel('Navn', { exact: true }).fill('Mikkel'); + await gæst.getByLabel(/Adgangskode/).fill('et langt kodeord til test'); + await gæst.getByRole('button', { name: 'Opret konto' }).click(); + await expect(gæst.getByRole('navigation', { name: 'Hovedmenu' })).toBeVisible(); + const noterURL = new URL('/noter', link).href; + + const sidebar = page.getByRole('navigation', { name: 'Hovedmenu' }); + await sidebar.getByRole('link', { name: 'Noter', exact: true }).click(); + + // Tre noter: en kæde på to, og en der ikke hænger sammen med noget. + for (const text of [ + 'Leveringstider for pakker', + 'Prisliste i kroner\nog [[Leveringstider for pakker]]', + 'Bankoplysninger som ingen skal se', + 'Aftale om levering\nse [[Prisliste i kroner]]' + ]) { + await page.getByRole('button', { name: 'Ny note' }).click(); + await page.getByRole('textbox', { name: 'Notens tekst' }).click(); + await page.keyboard.type(text); + await page.getByRole('textbox', { name: 'Notens tekst' }).blur(); + await expect(page.locator('footer .hint')).toHaveText('Gemt'); + } + + await page.locator('.notes button.row').filter({ hasText: /^Aftale om levering/ }).click(); + await page.locator('.sharewrap > button').click(); + const share = page.locator('.sharepanel'); + await share.locator('.addshare select').first().selectOption({ label: 'Mikkel' }); + await share.getByRole('button', { name: 'Tilføj' }).click(); + + // Sagt, ikke stiltiende: begge noter fulgte med, og panelet navngiver dem. + await expect(page.locator('.toast')).toContainText('de 2 noter'); + await expect(share.locator('.follows')).toContainText('Prisliste i kroner'); + await expect(share.locator('.follows')).toContainText('Leveringstider for pakker'); + + // Og hos ham: hele kæden kan læses, og linkene fører derhen. + await gæst.goto(noterURL); + await gæst.locator('.notes button.row').filter({ hasText: /^Aftale om levering/ }).click(); + await gæst + .getByRole('textbox', { name: 'Notens tekst' }) + .locator('a.notelink', { hasText: 'Prisliste i kroner' }) + .click(); + const hos = gæst.getByRole('textbox', { name: 'Notens tekst' }); + await expect(hos).toContainText('Prisliste i kroner'); + await hos.locator('a.notelink', { hasText: 'Leveringstider for pakker' }).click(); + await expect(gæst.getByRole('textbox', { name: 'Notens tekst' })).toContainText( + 'Leveringstider for pakker' + ); + // Men kun det, noten peger på. Resten er stadig ejerens alene. + await expect( + gæst.locator('.notes button.row').filter({ hasText: /^Bankoplysninger/ }) + ).toHaveCount(0); + + // Den fulgte adgang står som det, den er, i panelet under den note, den ramte. + // `button.row`, ikke bare knappen ved navn: panelet står åbent og har sin egen + // knap med den samme titel på. + await page.locator('.notes button.row').filter({ hasText: /^Prisliste i kroner/ }).click(); + await page.locator('.sharewrap > button').click(); + await expect(page.locator('.sharepanel .sharelist li').filter({ hasText: 'Mikkel' })).toContainText( + 'følger med' + ); + + // Taget tilbage følger den med tilbage. + await page.locator('.notes button.row').filter({ hasText: /^Aftale om levering/ }).click(); + await page.locator('.sharewrap > button').click(); + await page + .locator('.sharepanel .sharelist li') + .filter({ hasText: 'Mikkel' }) + .getByRole('button', { name: 'Fjern adgang' }) + .click(); + await gæst.goto(noterURL); + await expect( + gæst.locator('.notes button.row').filter({ hasText: /^Prisliste i kroner/ }) + ).toHaveCount(0); + + await ctx.close(); + expect(trouble).toEqual([]); +}); + /** * `` er kode, ikke kilde. * diff --git a/web/src/lib/locales/da.js b/web/src/lib/locales/da.js index e415eef..49193c3 100644 --- a/web/src/lib/locales/da.js +++ b/web/src/lib/locales/da.js @@ -34,6 +34,9 @@ export const da = { 'notes.shareRemove': 'Fjern adgang', 'notes.shareNobody': 'Ikke delt med nogen endnu.', 'notes.sharedToast': 'Delt med {name}.', + 'notes.sharedWithFollowers': 'Delt med {name} — og de {count} noter, den peger på.', + 'notes.shareFollows': 'Følger med:', + 'notes.shareVia': 'følger med “{note}”', 'notes.unsharedPerson': '{name} kan ikke længere se noten.', 'notes.shareOwnerOnly': 'Kun ejeren kan dele noten.', 'notes.save': 'Gem', diff --git a/web/src/lib/locales/en.js b/web/src/lib/locales/en.js index def0814..1dc6149 100644 --- a/web/src/lib/locales/en.js +++ b/web/src/lib/locales/en.js @@ -30,6 +30,9 @@ export const en = { 'notes.shareRemove': 'Remove access', 'notes.shareNobody': 'Not shared with anyone yet.', 'notes.sharedToast': 'Shared with {name}.', + 'notes.sharedWithFollowers': 'Shared with {name} — along with the {count} notes it points at.', + 'notes.shareFollows': 'Comes along:', + 'notes.shareVia': 'came along with “{note}”', 'notes.unsharedPerson': '{name} can no longer see the note.', 'notes.shareOwnerOnly': 'Only the owner can share the note.', 'notes.save': 'Save', diff --git a/web/src/routes/noter/+page.svelte b/web/src/routes/noter/+page.svelte index 49c6153..55c4fe5 100644 --- a/web/src/routes/noter/+page.svelte +++ b/web/src/routes/noter/+page.svelte @@ -192,6 +192,10 @@ // hentes kun for en note, man selv har lavet. let shares = $state([]); let shareCandidates = $state([]); + // De noter, denne note tager med sig, når den deles. Ejeren skal kunne se dem: + // en deling, der stille tager tre noter mere med, er rigtig og alligevel ikke + // til at overskue. + let shareFollows = $state([]); let sharePick = $state(''); let shareRole = $state('viewer'); let shareBusy = $state(false); @@ -223,6 +227,7 @@ // under den nye. shares = []; shareCandidates = []; + shareFollows = []; sharePick = ''; if (!id || selected.created_by !== app.user?.id) return; let alive = true; @@ -232,6 +237,7 @@ if (!alive || selected?.id !== id) return; shares = r.shares ?? []; shareCandidates = r.candidates ?? []; + shareFollows = r.follows ?? []; }) .catch(() => {}); return () => { @@ -246,10 +252,20 @@ const person = shareCandidates.find((p) => p.id === sharePick); shareBusy = true; try { - await api.shareNote(selected.id, sharePick, shareRole); + const r = await api.shareNote(selected.id, sharePick, shareRole); shares = [...shares, { user: person, role: shareRole }].sort((a, b) => byName(a.user, b.user)); shareCandidates = shareCandidates.filter((p) => p.id !== sharePick); - app.toast(t('notes.sharedToast', { name: person?.name ?? '' })); + shareFollows = r?.follows ?? shareFollows; + // Sagt frem for stiltiende. Delingen tog flere noter med, og den, der + // delte, skal vide hvor mange, uden at skulle læse panelet efter. + app.toast( + shareFollows.length + ? t('notes.sharedWithFollowers', { + name: person?.name ?? '', + count: String(shareFollows.length) + }) + : t('notes.sharedToast', { name: person?.name ?? '' }) + ); sharePick = ''; } catch (e) { app.toast(humanMessage(e)); @@ -1024,25 +1040,49 @@ > {sh.user.name} - - + + {#if sh.linked} + {t('notes.shareVia', { note: sh.via ?? '' })} + {:else} + + + {/if} {/each} {:else} {t('notes.shareNobody')} {/if} + {#if shareFollows.length} + +

+ {t('notes.shareFollows')} + {#each shareFollows as f, i (f.note_id)}{i < shareFollows.length - 1 ? ', ' : ''}{/each} +

+ {/if} {#if shareCandidates.length}