Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add search form on admin moderated content #3392

Merged
merged 5 commits into from
Aug 23, 2022
Merged

Add search form on admin moderated content #3392

merged 5 commits into from
Aug 23, 2022

Conversation

Jacek-202
Copy link
Contributor

@Jacek-202 Jacek-202 commented Mar 20, 2019 •
edited by javierm
Loading

References

Objectives

Add search form for all admin/hidden sections

Visual Changes

Screenshot from 2019-03-20 12-33-56

Notes

Added tsv vector colum to comments and proposal_notifications tables
Added 2 rake tasks to update tsv_vector

houndci-bot
houndci-bot reviewed Mar 20, 2019
View reviewed changes
app/models/proposal_notification.rb Outdated Show resolved Hide resolved
app/models/proposal_notification.rb Outdated Show resolved Hide resolved
app/models/comment.rb Outdated Show resolved Hide resolved
app/controllers/admin/proposal_notifications_controller.rb Outdated Show resolved Hide resolved
@javierm javierm changed the title Issue 3183 Add search form on admin moderated content Sep 10, 2019
@javierm javierm added this to Reviewing in Roadmap Sep 10, 2019
@javierm
Copy link
Member

javierm commented Oct 21, 2019

@Jacek-202 Thank you for this pull request 😄.

Sorry it's taking us so long to review it 🙏. It looks like Travis is reporting a failure in one test, and Hound is also reporting some issues 🤔.

Would you like to continue working on this pull request? If so, thanks a lot 😉. Could you have a look at these issues?

@javierm javierm moved this from Reviewing to Doing in Roadmap Oct 21, 2019
@Jacek-202
Copy link
Contributor Author

Thanks for comment @javierm .
I saw that part of this task is already merged. I will try to review what need to be done in next weeks.

@javierm
Copy link
Member

javierm commented Nov 13, 2019

@Jacek-202 Cool! 😄 If part of this pull request is already merged, you might want to run git rebase against our current master branch (beware you'll have to solve a few conflicts. Sorry for that! 🙏) and then push the updated changes with git push -f.

@javierm
Copy link
Member

javierm commented Feb 28, 2020

Hi, @Jacek-202, how are you doing?

Are you still working on this pull request? 🤔

@Jacek-202
Copy link
Contributor Author

Hi @javierm, sorry that i didn't update it. I will check this issues and prepare pull request in 3-4 weeks (from now) :) .

@javierm javierm added this to Doing in Consul Democracy Mar 11, 2020
@javierm javierm removed this from Pending in Roadmap Mar 11, 2020
houndci-bot
houndci-bot reviewed Apr 13, 2020
View reviewed changes
app/models/proposal_notification.rb Outdated Show resolved Hide resolved
app/models/proposal_notification.rb Outdated Show resolved Hide resolved
app/models/comment.rb Outdated Show resolved Hide resolved
app/controllers/admin/proposal_notifications_controller.rb Outdated Show resolved Hide resolved
app/controllers/admin/proposal_notifications_controller.rb Outdated Show resolved Hide resolved
app/controllers/admin/comments_controller.rb Outdated Show resolved Hide resolved
app/controllers/admin/comments_controller.rb Outdated Show resolved Hide resolved
@Jacek-202 Jacek-202 force-pushed the issue-3183 branch 4 times, most recently from c32330f to ccf72ec Compare April 16, 2020 16:05
@Jacek-202
Copy link
Contributor Author

Hi @javierm
Please make initial review :)

@javierm javierm moved this from Doing to Reviewing in Consul Democracy Apr 17, 2020
@javierm javierm added the Admin label Nov 2, 2021
@javierm javierm force-pushed the issue-3183 branch 2 times, most recently from 126d695 to c1636ce Compare December 3, 2021 16:49
@javierm
Copy link
Member

javierm commented Dec 3, 2021 •
edited
Loading

@Jacek-202 Sorry to make you wait for a year and a half 🙏. Only now we're finally catching up with the last of the pull requests pending review.

I've pushed some changes in order to make the pull request up to date with the latest master branch.

Would you like to keep working on this pull request, or shall we take it from here? I understand it's really unfair to ask you to add changes after such a long time, so if you'd like to stop working working on this pull request, just say so 😉.

If you're interested in continuing, here's the initial review you asked for... 20 months ago 😅.

  • Would it be possible to add some tests for this feature? 🙏
  • The text "Check the content moderated by the moderators, and confirm if the moderation has been done correctly." seems to appear in sections like "debates" and "proposals" as well (outside the moderated content) 🤔

@javierm javierm moved this from Reviewing to Doing in Consul Democracy Dec 3, 2021
@javierm javierm self-assigned this Mar 23, 2022
@javierm javierm force-pushed the issue-3183 branch 4 times, most recently from 0dd8250 to 3ed6d9d Compare August 9, 2022 17:48
@javierm javierm force-pushed the issue-3183 branch 4 times, most recently from 0016791 to 84aa293 Compare August 9, 2022 18:18
@javierm javierm moved this from Doing to Testing in Consul Democracy Aug 9, 2022
@taitus taitus self-assigned this Aug 20, 2022
@javierm javierm force-pushed the issue-3183 branch 3 times, most recently from 1076115 to 239f637 Compare August 23, 2022 12:30
javierm and others added 4 commits August 23, 2022 14:30
@javierm
Order filters the same way in all hidden content
3e50b7c 
We were doing it differently for investments.
@javierm
Extract concern to share hidden content code
f6fefde
@javierm
Fix calculating tsvector on hidden records
e66b968 
We introduced this bug in commit 55d3395, since we didn't take hidden
records into consideration.

I've tried to use `update_column` to simplify the code, but got a syntax
error `unnamed portal parameter` and didn't find how to fix it.
@Jacek-202 @javierm
Add search form for hidden content
2af7e32 
Added search for comments and proposal_notifications, added tsv column
for search and rake tasks to update/create tsv vector.
@javierm
Keep filters in admin search
92941d3 
While this bug was already present in the general admin search, the
combination of both search and filters was very uncommon. I've only
found this combinations in the users section, where you've got the
"erased" filter, but in this case searching for erased users doesn't
really make sense since their username and email have been deleted and
so there's nothing to find.

So the hidden content seemed to be the only affected section. However,
we're adding the field to every section so we don't have to make sure we
add it when we need it (like we did in the SDGManagement section).
@javierm javierm merged commit a4b9d88 into consuldemocracy:master Aug 23, 2022
Consul Democracy automation moved this from Testing to Release 1.6.0 Aug 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@houndci-bot houndci-bot houndci-bot left review comments

@taitus taitus taitus approved these changes

Assignees

@taitus taitus

@javierm javierm

@Jacek-202 Jacek-202

Labels
Admin
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Search form on admin moderated content
4 participants
@Jacek-202 @javierm @taitus @houndci-bot