New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Forum show deleted threads #2151

Merged
merged 14 commits into from Jun 25, 2018

Conversation

Projects
None yet
5 participants
@scopeInfinity
Copy link
Member

scopeInfinity commented Jun 13, 2018

No description provided.

@bmcutler bmcutler requested review from ropern123 and andrewaikens87 Jun 13, 2018

@ropern123
Copy link
Contributor

ropern123 left a comment

I was able to reach an error:

Enter the edit post field,
then make changes but don't close it
then click the delete button
then confirm deletion in first popup
then don't leave in second popup.

This deletes the thread on the backend, but creates errors on the front end (i.e. page stays open on deleted thread).

@scopeInfinity

This comment has been minimized.

Copy link
Member Author

scopeInfinity commented Jun 16, 2018

@ropern123
Yup, above the procedure deletes the thread and keep it open on frontend.
However, the issue is related to how post edit form are implemented for now.
I will be adding it in todo list.

@scopeInfinity scopeInfinity changed the title [WIP] Forum show deleted threads Forum show deleted threads Jun 16, 2018

@MasterOdin

This comment has been minimized.

Copy link
Member

MasterOdin commented Jun 18, 2018

So on my understanding of this, we want to make it so that if a user decides to view deleted threads, if we goes away from the page, and goes back, we again want to show the user the deleted threads?

It would probably make sense to use cookies instead of $_SESSION to maintain this so that it survives closing/opening the browser.

@ropern123
Copy link
Contributor

ropern123 left a comment

It does not appear to be displaying the deleted threads on the thread list when Show Deleted Threads is clicked.

@andrewaikens87
Copy link
Member

andrewaikens87 left a comment

The deleted threads aren't populating for me either. I deleted a few threads and toggled show delete threads and got this:
error
bad

@scopeInfinity

This comment has been minimized.

Copy link
Member Author

scopeInfinity commented Jun 21, 2018

Yah, It got messed up during one of the merge, I might have missed it.
It's working now.

@ropern123
Copy link
Contributor

ropern123 left a comment

Seems to be working pretty well, only minor issue I encountered was that while nobody can post replies to a deleted thread directly, it is still possible to post replies to deleted posts as an instructor (or even a student with the right ajax call), which leaves the reply "dangling" from a deleted post.

@ropern123
Copy link
Contributor

ropern123 left a comment

Being able to reply to deleted posts does not seem to be a problem specific to this PR; I'll make a separate issue for it.

@andrewaikens87
Copy link
Member

andrewaikens87 left a comment

Works well, the only thing that I noticed was if there exists a deleted announcement and I navigate to the
index.php?semester=s18&course=sample&component=forum&page=view_thread it will default to viewing the first non announcement which is different behavior than when not viewing deleted threads, but this should be fine.

@bmcutler bmcutler merged commit 2d5b580 into master Jun 25, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@bmcutler bmcutler deleted the forum_thread_show_deleted branch Jun 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment