LB-1230: Added ability to edit comment after pinning a Listen

[lzzy12]

Problem

Fixes this Ticket:
https://tickets.metabrainz.org/browse/LB-1230

Solution

Added an extra action for pinned listen:

[pep8speaks]

Hello @lzzy12! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-05-29 14:52:49 UTC

[MonkeyDo]

Thanks for opening a PR! It's looking quite good !

I did a first round of review without running the code, but it's looking functional already.
Some questions about return value and statuses below:

[lzzy12]

Hi, thanks for your feedbacks. I will work on them

[lzzy12]

@MonkeyDo I have pushed an update. Please have a look when you feel better :)

[MonkeyDo]

Thanks for the modifications, I have a couple more comments but it's looking nice !

[MonkeyDo]

Thanks for those changes, the code looks great now !

I deployed the PR to our test server, and it's working nicely! I was able to update a comment without any issue, nice work !

Through testing I do have two more pieces of feedback from a user perspective. Sorry about not realizing sooner !

• It would be useful to have the current pin comment in the textarea when I open the modal to edit the pin. Currently to update a typo I would have to copy the comment first. Left some comments about that below
• I would want the UI to update so that the new text content is shown on the pin card on the page. Also left some comments to that effect, but basically we can consider the call modal.show( as a promise and make the modal return a value when the API call succeeds (in this case the updated text).

[lzzy12]

Actually the current behavior was inlined to what happens when we pin a track. It only shows up in pin when the user refreshes. I have observed this behavior at multiple places in the webapp. That was the reason I tried to keep it inlined to that behavior.
I will work on it to implement the suggested changes

[MonkeyDo]

Actually the current behavior was inlined to what happens when we pin a track. It only shows up in pin when the user refreshes. I have observed this behavior at multiple places in the webapp. That was the reason I tried to keep it inlined to that behavior.
I will work on it to implement the suggested changes

Yes, I didn't want to muddy the waters by mentioning it, but I did realize the pin is not updated on the page when pinning a track, which is definitely not great UX :)

I think since you are working on that feature, and since you noticed it yourself, you will be in a great position to fix that too in your next PR, if you are interested in doing that.

[MonkeyDo]

Hi @lzzy12 !
Just checking in to see if you have some time to finish this PR, or if you would rather pass the baton to someone else.
Either way is fine by me, please let me know :)

[lzzy12]

Sorry got caught up with something. Will be finishing it this weekend

[MonkeyDo]

I took the liberty to finish this off, hope you don't mind :)