bellroy / lesswrong

Less Wrong platform
http://lesswrong.org/
Other
49 stars 23 forks source link

Add the ability to submit links. Fixes #548 #575

Closed jglamine closed 8 years ago

jglamine commented 8 years ago

This turned out to be non-trivial. I'm not sure what tricycle's release process is like, but someone other than me should probably do some additional regression testing.

I suggest using this link to view the diff while ignoring whitespace changes: https://github.com/tricycle/lesswrong/pull/575/files?w=1

jglamine commented 8 years ago

home

Link posts are automatically prefixed with "[Link]". There is a new "Submit a new link" button on the sidebar.

jglamine commented 8 years ago

submit-post The submit page has two tabs.

jglamine commented 8 years ago

submit-link

jglamine commented 8 years ago

link-page Clicking on "comments" brings you to this page. The title links to the actual link url.

jglamine commented 8 years ago

edit-link The edit link page allows you to change the title but not the url.

wezm commented 8 years ago

I would be great if you could take some time to increase your confidence that this won't break the site. At the very least there is a manual test script that goes through the core activities of the system:

https://github.com/tricycle/lesswrong/wiki/Hacking-on-Less-Wrong#manual-tests

Additionally, since this changes the link model, which is the core model of the application it will be worth poking at all the places links and comments are shown such as the main listings, sidebar, RSS feeds, user messages, the front page, user profile page listings.

Wesley Moore Senior Developer ph: +61 3 9024 2760 TrikeApps http://trikeapps.com/

On 18 Jul 2016, at 12:30 PM, James Lamine notifications@github.com wrote:

This turned out to be non-trivial. I'm not sure what tricycle's release process is like, but someone other than me should probably do some additional regression testing.

You can view, comment on, or merge this pull request online at:

https://github.com/tricycle/lesswrong/pull/575 https://github.com/tricycle/lesswrong/pull/575 Commit Summary

Add the ability to submit links. Fixes #548 File Changes

M r2/r2/controllers/api.py https://github.com/tricycle/lesswrong/pull/575/files#diff-0 (82) M r2/r2/controllers/front.py https://github.com/tricycle/lesswrong/pull/575/files#diff-1 (12) M r2/r2/lib/menus.py https://github.com/tricycle/lesswrong/pull/575/files#diff-2 (9) M r2/r2/lib/pages/pages.py https://github.com/tricycle/lesswrong/pull/575/files#diff-3 (65) A r2/r2/lib/tabs.py https://github.com/tricycle/lesswrong/pull/575/files#diff-4 (7) M r2/r2/models/link.py https://github.com/tricycle/lesswrong/pull/575/files#diff-5 (110) M r2/r2/public/static/main.css https://github.com/tricycle/lesswrong/pull/575/files#diff-6 (60) M r2/r2/public/static/utils.js https://github.com/tricycle/lesswrong/pull/575/files#diff-7 (26) M r2/r2/templates/link.html https://github.com/tricycle/lesswrong/pull/575/files#diff-8 (57) M r2/r2/templates/newlink.html https://github.com/tricycle/lesswrong/pull/575/files#diff-9 (127) A r2/r2/templates/tabs.html https://github.com/tricycle/lesswrong/pull/575/files#diff-10 (18) M r2/test.ini https://github.com/tricycle/lesswrong/pull/575/files#diff-11 (29) Patch Links:

https://github.com/tricycle/lesswrong/pull/575.patch https://github.com/tricycle/lesswrong/pull/575.patch https://github.com/tricycle/lesswrong/pull/575.diff https://github.com/tricycle/lesswrong/pull/575.diff — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/tricycle/lesswrong/pull/575, or mute the thread https://github.com/notifications/unsubscribe-auth/AABVG-Hmu1nUNHVvqvaQR7v0u4uty4BRks5qWuVigaJpZM4JOX9D.

jglamine commented 8 years ago

Good idea, will do.

jglamine commented 8 years ago

I fixed a few issues with rss feeds, side-bar, and front page links not being correct (linking to the url instead of the lesswrong post).

I ran through the manual tests.

This should be good to go.

wezm commented 8 years ago

I have taken some time to review this and have identified a few things that need addressing:

jglamine commented 8 years ago

Thank for catching these issues. I'll reproduce them and update the PR with fixes.

jglamine commented 8 years ago

I was able to reproduce your bugs. Two of them were a bit tricky to reproduce. The do not appear to have anything to do with captchas. Here are the steps:

Bug 1 - Links turn into articles:

  1. Click "Create new article"
  2. Switch to the link tab.
  3. Fill in info, submit.

Expected behavior: brings you to the page for a newly created link Actual behavior: brings you to the page for a newly created article

Bug 2 - article editor is too small:

  1. Click "Submit a new link"
  2. Switch to the article tab

Expected behavior: the text editor is 500px tall. Actual behavior: the text editor is 84px tall.

jglamine commented 8 years ago

PR updated.

I was not able to reproduce the "That link has already been submitted." flash message. If it still happens, please provide me with steps to reproduce and let me know what browser you're using.

Changes are as follows:

When you get a chance, I'd love to get your feedback on this.

jglamine commented 8 years ago

Also note that there is not much validation around URLs. For example, links don't have to be valid urls. I don't think it's that important because people won't upvote broken links. If it turns out we really want better validation, we can always do it in another issue.

Also note that it's not possible to edit a link after submitting it, only the title. If we want admins to be able to edit links, we can do that as a separate issue. They can still delete them or edit the title, which should be enough for now.

wezm commented 8 years ago

I was not able to reproduce the "That link has already been submitted." flash message. If it still happens, please provide me with steps to reproduce and let me know what browser you're using.

I think this was only being shown due to the inline validation not working. Now that it is fixed, it doesn't show up for me either.

wezm commented 8 years ago

This looks good now. I'll deploy in the morning. For future reference I'd prefer you didn't force update an existing PR. If you add commits to address feedback it's easier for me to review just those changes instead of needing to review all changes again.

wezm commented 8 years ago

Deployed