slidewiki-platform icon indicating copy to clipboard operation
slidewiki-platform copied to clipboard

Published 20 hours ago • verified publisher icon slidewiki

Swik 1440 improve slide links in notifications

Open dpaun opened this issue 6 years ago • 7 comments

Currently, slide links in notifications point to slide-view, which is not good. Using the recently added activity parameter, content_root_id, slide links (in newly created notifications, which contain content_root_id) will point to proper path within a deck (deck-view + slide).

dpaun avatar Oct 23 '18 09:10 dpaun

Haven't tested it yet, but I can see you also include the spath, calculating it ? Very interesting and maybe we can use it for other things, however there are issues with this parameter in our URLs, in combination with including the revision for decks / slides. These can lead to 404 pages, for an ever-changing deck. If the deck revision is not the latest, then the URL will never break, but if we are not, then those things (the spath and the slide revision) can change.

So what I am asking, can we simply not include the spath and the slide revision? You can keep the deck revision for now. Or are there other considerations for that?

To be clear, we can store things in services however we want, I'm just talking about the URLs.

kprist avatar Oct 23 '18 12:10 kprist

Can I use urls like these: /deck/ {deck id }/slide/ {slide id} ? Without the path part in it? It seems to work. But it doesn't work if there is no slide revision number.

dpaun avatar Oct 23 '18 13:10 dpaun

You should be able to do that, in theory. Can you give me an example where removing the slide revision fails ?

kprist avatar Oct 23 '18 14:10 kprist

Yes. https://platform.experimental.slidewiki.org/deck/4676-4/slide/41138 fails https://platform.experimental.slidewiki.org/deck/4676-4/slide/41138-2 works

dpaun avatar Oct 23 '18 14:10 dpaun

Hmm this looks like a bug / regression, let me check it

kprist avatar Oct 23 '18 15:10 kprist

Not exactly a bug or regression, platform has added features where spath and revisions are needed again, so path generation is ok for now. It's a bit overkill we need to do a whole decktree request however, so I opened ticket https://slidewiki.atlassian.net/browse/SWIK-2516 and branches in deck and platform wherein the idea is similar to what you do here, i.e. I recreate the spath and revisions, but for the whole page, not just for the sake of the notifications links.

If you think it's important to merge, let me know. Since it's not UI-related, it could wait.

On another note, please also apply the same URL strategy not just for slides, but also for subdecks, where applicable.

kprist avatar Oct 24 '18 00:10 kprist

It can wait, I agree. I will apply it for subdecks in the next commit.

dpaun avatar Oct 24 '18 06:10 dpaun