Remove references to "place" and "tool" node types

[jywarren]

These are now completely gone from the database, and we can remove that code. All those types have been converted to "page" and we can just use "page" in those queries now.

There are instances in a few places, including:

https://github.com/publiclab/plots2/blob/master/app/controllers/wiki_controller.rb#L267

for example. Other file that include these are:

https://github.com/publiclab/plots2/blob/master/app/controllers/notes_controller
https://github.com/publiclab/plots2/blob/master/app/controllers/tag_controller

https://github.com/publiclab/plots2/blob/master/app/models/drupal_node.rb
https://github.com/publiclab/plots2/blob/master/app/models/drupal_users.rb
https://github.com/publiclab/plots2/blob/master/app/models/drupal_tag.rb

There may be more instances as well. But there are none in the fixtures, so in removing all these, tests should still pass.

[nickstaggs]

@jywarren I can take this one. Just so I understand what you want, the original line is:

.where("node_revisions.status = 1 AND node.status = 1 AND (type = 'page' OR type = 'tool' OR type = 'place')")

and the final product should be:

.where("node_revisions.status = 1 AND node.status = 1 AND type = 'page")

for all instances where place and tool node types occur?

[jywarren]

Yes -- that's right! And note to self that we'll have to triple-check that
all tool and place pages have been converted over before merging this.
Thanks!

On Wed, Sep 21, 2016 at 2:35 PM, Nick Staggs notifications@github.com
wrote:

@jywarren https://github.com/jywarren I can take this one. Just so I
understand what you want, the original line is:

.where("node_revisions.status = 1 AND node.status = 1 AND (type = 'page' OR type = 'tool' OR type = 'place')")

and the final product should be:

.where("node_revisions.status = 1 AND node.status = 1 AND type = 'page")

for all instances where place and tool node types occur?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#714 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABfJ_wgoMEBuyDCNY0mkShKWGphNX8Nks5qsXj9gaJpZM4JpLpI
.

[nickstaggs]

@jywarren could you assign this to me?

[jywarren]

Absolutely -- done! Thanks!!

[jywarren]

Hi, just checking if you've gotten stuck on this at all, or if I could help in any way? Thanks!

[grvsachdeva]

@jywarren is this issue solved?

[jywarren]

Not quite, but it could make for some good FTOs! https://github.com/publiclab/plots2/blob/b09034c25d06bb9b1533b56d9c8a3eda5a57a9f6/app/controllers/wiki_controller.rb#L320 https://github.com/publiclab/plots2/blob/205653b9e10eed21716f3f61281bc9c4574933f2/app/services/search_service.rb#L55 https://github.com/publiclab/plots2/blob/6227e95de07861677f6586d9ec760b1c0c7ac525/app/models/tag.rb#L116 https://github.com/publiclab/plots2/blob/66ec52188ae7b95a8899a64471970b8a2d90e248/app/controllers/tag_controller.rb#L450 I believe that's all of them! They should able be convertable to just "page". Thanks!

…

On Mon, Mar 25, 2019 at 12:27 PM Gaurav Sachdeva ***@***.***> wrote: @jywarren <https://github.com/jywarren> is this issue solved? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#714 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABfJ1jjY4c37bZmvmni7cUB6IcgUsX7ks5vaPjOgaJpZM4JpLpI> .

[grvsachdeva]

Yes, it can!

[swathi-0901]

@jywarren can I work on this issue?

[swathi-0901]

@jywarren can I work on this issue?

@jywarren can u assign this to me?

[jywarren]

We'd love your help -- no need to assign, and sorry for the slow reply!

…

On Sat, Nov 9, 2019 at 5:07 AM Swathi Kasikala ***@***.***> wrote: @jywarren <https://github.com/jywarren> can I work on this issue? @jywarren <https://github.com/jywarren> can u assign this to me? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#714?email_source=notifications&email_token=AAAF6J2DNLDTAM3SEC4JOG3QS2DSZA5CNFSM4CNEXJEKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDUCNYI#issuecomment-552085217>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAAF6J52DYEIEMEVM7MQ3G3QS2DSZANCNFSM4CNEXJEA> .

[cesswairimu]

UPDATE

The file links currently that still have this references are:

https://github.com/publiclab/plots2/blob/main/app/controllers/wiki_controller.rb
https://github.com/publiclab/plots2/blob/main/app/controllers/notes_controller.rb
https://github.com/publiclab/plots2/blob/main/app/models/node.rb
https://github.com/publiclab/plots2/blob/main/app/services/search_service.rb
e.g
https://github.com/publiclab/plots2/blob/main/app/controllers/wiki_controller.rb#L377 would ideally be change from
.where("status = 1 AND nid != 259 AND (type = 'page' OR type = 'tool' OR type = 'place') AND cached_likes >= 0")
to
.where("status = 1 AND nid != 259 AND type = 'page' AND cached_likes >= 0")

This could potentially be broken down to to ftos or worked on as one.

please note: some tests may fail after this and that will need to be fixed with this PR...No worries if you are new to testing we are willing to help once the changes are done. thanks

[anirudhprabhakaran3]

@jywarren @cesswairimu should I make this into a bunch of FTOs? Or should I work on it?

[cesswairimu]

Hi @anirudhprabhakaran3 , either its great and will be highly appreciated..your call. thanks

[anirudhprabhakaran3]

I think @KSVSC is working on this, if that's the case I'll take up some other issue!

[KSVSC]

I think @KSVSC is working on this, if that's the case I'll take up some other issue!

Yes, I am working on this. Thanks :)

[KSVSC]

@jywarren @cesswairimu @TildaDares Hi! I have opened a PR for this, kindly review it :)

[KSVSC]

UPDATE

The file links currently that still have this references are:

https://github.com/publiclab/plots2/blob/main/app/controllers/wiki_controller.rb
https://github.com/publiclab/plots2/blob/main/app/controllers/notes_controller.rb
https://github.com/publiclab/plots2/blob/main/app/models/node.rb
https://github.com/publiclab/plots2/blob/main/app/services/search_service.rb
e.g
https://github.com/publiclab/plots2/blob/main/app/controllers/wiki_controller.rb#L377 would ideally be change from
.where("status = 1 AND nid != 259 AND (type = 'page' OR type = 'tool' OR type = 'place') AND cached_likes >= 0")
to
.where("status = 1 AND nid != 259 AND type = 'page' AND cached_likes >= 0")

This could potentially be broken down to to ftos or worked on as one.

please note: some tests may fail after this and that will need to be fixed with this PR...No worries if you are new to testing we are willing to help once the changes are done. thanks

Hi, @cesswairimu. I believe that Line nos 22-28 needs to be removed too in the file https://github.com/publiclab/plots2/blob/main/app/controllers/legacy_controller.rb#L22-#L28. Please confirm and review my PR for this issue. Thanks!

[cesswairimu]

Hi @KSVSC on this https://github.com/publiclab/plots2/blob/main/app/controllers/legacy_controller.rb#L22-#L28 that is fine we can leave it as is...bacuse the request already redirects to /wiki

what we would like to remove is anywhere we are filtering nodes with the type 'page' OR 'tool'.

e.g select * from nodes where type='tool' -- we shouldn't have any query that has this type of filter...what do you think?

[KSVSC]

@cesswairimu, Agreed! I made the required modifications to my PR. Thanks!

[cesswairimu]

great 🎉 , thanks...reviewing in a few