Skip to content
This repository has been archived by the owner on Oct 24, 2022. It is now read-only.

Commit

Permalink
Reharvestable Items from Items missing Topics page
Browse files Browse the repository at this point in the history
- adds a method to explictily check for `Any/None` topics and uses that in determing problem Items
- adds a PullKnownItem link on the Items missing Topics page to allow sysadmins to attempt
a reharvest
 - note: this does not allow reharvesting from Publishers with multiple Harvests at this time
- allows sysadmins to delete an Item entirely from the Items missing Topics page if they determine
that is an appropriate course of action for the Item (i.e. they happen to know it is not
relevant to hub Subscribers or it was retracted upstream or something).

closes #255
  • Loading branch information
JPrevost committed Jul 10, 2015
1 parent 54292f0 commit 1d24701
Show file tree
Hide file tree
Showing 9 changed files with 186 additions and 8 deletions.
7 changes: 6 additions & 1 deletion app/controllers/ItemController.scala
Expand Up @@ -29,6 +29,11 @@ object ItemController extends Controller with Security {
}
}

def delete(id: Int) = isAdmin { identity => implicit request =>
Item.delete(id)
Ok("Item deleted")
}

private def itemBrowseTopic(id: Int, page: Int)(implicit request: Request[AnyContent]): Result = {
Topic.findById(id).map( topic =>
Ok(views.html.item.browse(id, topic.pagedItems(page, 10), "topic", topic.name, page, topic.itemCount))
Expand All @@ -42,7 +47,7 @@ object ItemController extends Controller with Security {
}

def itemsWithNoTopics = isAdmin { identity => { implicit request =>
Ok(views.html.item.notopics(Item.allMissingMetadata))
Ok(views.html.item.notopics(Item.allWithCatalogErrors))
}
}

Expand Down
18 changes: 18 additions & 0 deletions app/models/Item.scala
Expand Up @@ -285,6 +285,24 @@ object Item {
}
}

def allWithCatalogErrors: List[Item] = {
DB.withConnection { implicit c =>
SQL("""
SELECT item.*
FROM item
WHERE id NOT IN (
SELECT DISTINCT item.id
FROM item
LEFT JOIN item_topic on item.id = item_topic.item_id
LEFT JOIN topic on topic.id = item_topic.topic_id
WHERE topic.tag = 'any'
OR topic.tag = 'none'
)
ORDER BY created DESC
""").as(item *)
}
}

def findByKey(key: String): Option[Item] = {
DB.withConnection { implicit c =>
SQL("select * from item where obj_key = {key}").on('key -> key).as(item.singleOpt)
Expand Down
10 changes: 10 additions & 0 deletions app/models/Publisher.scala
Expand Up @@ -48,6 +48,16 @@ case class Publisher(id: Int, // DB key
}
}

def harvests = {
DB.withConnection { implicit c =>
SQL("""
SELECT *
FROM harvest
WHERE publisher_id = {id}
""").on('id -> id).as(Harvest.harv *)
}
}

def harvestCount = {
DB.withConnection { implicit c =>
SQL("select count(*) from harvest where publisher_id = {id}").on('id -> id).as(scalar[Long].single)
Expand Down
23 changes: 18 additions & 5 deletions app/views/item/notopics.scala.html
Expand Up @@ -2,22 +2,35 @@
* Page to display Items missing Topics (which is an error state) *
* Copyright (c) 2015 MIT Libraries *
*****************************************************************************@
@(items: List[Item])(implicit request: play.api.mvc.RequestHeader)
@(items: List[Item])(implicit request: play.api.mvc.RequestHeader)

@layout.main("Items missing Topics - TopicHub") {
<div class="page-header">
<h2>Items missing Topics</h2>
<p>These items are in an error state as all Items should have at least
one topic assigned by a successful cataloger worker.</p>
<p>At this time we have no method to clean up these records from this interface.
Once we know what likely steps we should take, we can add that.</p>
</div>
<ul class="list-group">
@items.map { item =>
<li class="list-group-item">
@HubUtils.fmtPreciseDateTime(item.created)
<a href="@routes.ItemController.item(item.id)">View Local record</a>
<a href="@item.location.substring(7)">View Remote Record</a>
<div class="btn-group" role="group">
<a class="btn btn-default" href="@routes.ItemController.item(item.id)">View Local</a>

<a class="btn btn-default" href="@item.location.substring(7)">View Remote</a>

@if(Publisher.findById(Collection.findById(item.collectionId).get.publisherId).get.harvestCount == 1) {
<a id="reharvest-@item.id" class="btn btn-default" href="@routes.Application.pullKnownItem(
item.collectionId,
Publisher.findById(Collection.findById(item.collectionId).get.publisherId).get.harvests.head.id,
item.objKey,
true)">Attempt Reharvest</a>
} else {
<a class="btn btn-default disabled">Unable to determine Harvest</a>
}

<a id="delete-@item.id" class="btn btn-default" href="@routes.ItemController.delete(item.id)">Delete</a>
</div>
</li>
}
</ul>
Expand Down
2 changes: 1 addition & 1 deletion app/views/static/workbench.scala.html
Expand Up @@ -17,7 +17,7 @@ <h3>Tools</h3>
</a>

<a id="sidenav_notopic_items" href="@routes.ItemController.itemsWithNoTopics" class="list-group-item">
Items with no Topics <span class="badge">@{ Item.allMissingMetadata.size }</span></a>
Items with no Topics <span class="badge">@{ Item.allWithCatalogErrors.size }</span></a>
}

<a id="sidenav_schemes" href="@routes.Application.schemes" class="list-group-item">Schemes</a>
Expand Down
1 change: 1 addition & 0 deletions conf/routes
Expand Up @@ -100,6 +100,7 @@ GET /item/package/:id controllers.ItemController.itemPackage(id:
GET /item/deposit/:id controllers.ItemController.itemDeposit(id: Int)
GET /item/mets/:id controllers.ItemController.itemMets(id: Int)
GET /items/missingtopics controllers.ItemController.itemsWithNoTopics
GET /item/delete/:id controllers.ItemController.delete(id: Int)

# Topic pages
GET /topics controllers.Application.topics
Expand Down
64 changes: 63 additions & 1 deletion test/integration/ItemPagesSpec.scala
@@ -1,13 +1,14 @@
import org.specs2.mutable._
import org.specs2.runner._
import java.util.Date

import play.api.test._
import play.api.test.Helpers._
import org.fest.assertions.Assertions.assertThat
import play.api.Application
import play.api.Play
import play.api.Play.current
import models.{ Channel, Collection, ContentType, Item, Publisher, ResourceMap, Scheme,
import models.{ Channel, Collection, ContentType, Harvest, Item, Publisher, ResourceMap, Scheme,
Subscriber, Topic, User }

/**
Expand Down Expand Up @@ -200,6 +201,17 @@ class ItemPagesSpec extends Specification {
assertThat(browser.title()).isEqualTo("Error - TopicHub")
browser.pageSource must contain("You are not authorized")
}

"deleting an item is denied" in new WithBrowser(
app = FakeApplication(additionalConfiguration = inMemoryDatabase())) {
User.make("user", "user@example.com", "analyst", "https://oidc.mit.edu/current_user")
item_factory(1)
browser.goTo("http://localhost:" + port + "/login")
browser.$("#openid").click
browser.goTo("http://localhost:" + port + "/item/delete/1")
assertThat(browser.title()).isEqualTo("Error - TopicHub")
browser.pageSource must contain("You are not authorized")
}
}

"as a sysadmin" should {
Expand All @@ -213,6 +225,56 @@ class ItemPagesSpec extends Specification {
browser.goTo("http://localhost:" + port + "/items/missingtopics")
assertThat(browser.title()).isEqualTo("Items missing Topics - TopicHub")
}

"reharvest link displays for Items from a Publisher with a single Harvest" in new WithBrowser(
app = FakeApplication(additionalConfiguration = inMemoryDatabase())) {
User.make("user", "user@example.com", "sysadmin", "https://oidc.mit.edu/current_user")
item_factory(1)
Harvest.create(1, "name", "protocol", "http://www.example.com", "http://example.org", 1, new Date)
browser.goTo("http://localhost:" + port + "/login")
browser.$("#openid").click
browser.goTo("http://localhost:" + port + "/items/missingtopics")
browser.pageSource must contain("Attempt Reharvest")
browser.pageSource must contain("reharvest-1")
browser.pageSource must not contain("Unable to determine Harvest")
}

"reharvest link not shown for Items from a Publisher with multiple Harvests" in new WithBrowser(
app = FakeApplication(additionalConfiguration = inMemoryDatabase())) {
User.make("user", "user@example.com", "sysadmin", "https://oidc.mit.edu/current_user")
item_factory(1)
Harvest.create(1, "name", "protocol", "http://www.example.com", "http://example.org", 1, new Date)
Harvest.create(1, "name2", "protocol", "http://www.example.com", "http://example.org", 1, new Date)
browser.goTo("http://localhost:" + port + "/login")
browser.$("#openid").click
browser.goTo("http://localhost:" + port + "/items/missingtopics")
browser.pageSource must not contain("Attempt Reharvest")
browser.pageSource must not contain("reharvest-1")
browser.pageSource must contain("Unable to determine Harvest")
}

"reharvest link not shown for Items from a Publisher with no Harvests" in new WithBrowser(
app = FakeApplication(additionalConfiguration = inMemoryDatabase())) {
User.make("user", "user@example.com", "sysadmin", "https://oidc.mit.edu/current_user")
item_factory(1)
browser.goTo("http://localhost:" + port + "/login")
browser.$("#openid").click
browser.goTo("http://localhost:" + port + "/items/missingtopics")
browser.pageSource must not contain("Attempt Reharvest")
browser.pageSource must not contain("reharvest-1")
browser.pageSource must contain("Unable to determine Harvest")
}

"deleting an item is allowed" in new WithBrowser(
app = FakeApplication(additionalConfiguration = inMemoryDatabase())) {
User.make("user", "user@example.com", "sysadmin", "https://oidc.mit.edu/current_user")
item_factory(1)
browser.goTo("http://localhost:" + port + "/login")
browser.$("#openid").click
browser.goTo("http://localhost:" + port + "/items/missingtopics")
browser.$("#delete-1").click
browser.pageSource must contain("Item deleted")
}
}
}
}
45 changes: 45 additions & 0 deletions test/unit/ItemSpec.scala
Expand Up @@ -102,6 +102,51 @@ class ItemSpec extends Specification {
}
}

"#allWithCatalogErrors" in {
running(FakeApplication(additionalConfiguration = inMemoryDatabase())) {
val u = User.make("bob", "bob@example.com", "pass", "roley")
val ct = ContentType.make("tag", "label", "desc", Some("logo"))
val rm = ResourceMap.make("tag", "desc", Some("swordurl"))
val p = Publisher.make(u.id, "pubtag", "pubname", "pubdesc", "pubcat",
"pubstatus", Some(""), Some(""))
val col = Collection.make(p.id, ct.id, rm.id, "coll", "desc", "open")
val i1 = Item.make(col.id, ct.id, "loc", "scoap3:asdf:123")
val i2 = Item.make(col.id, ct.id, "loc", "scoap3:asdf:456")

val s = Scheme.make("tag", "gentype", "cat", "desc", Some("link"), Some("logo"))
val t = Topic.make(s.id, "tag", "abstract")
val t_any = Topic.make(s.id, "any", "Item with some topics")
val t_none = Topic.make(s.id, "none", "Item with no topics")
val item_list1 = Item.allWithCatalogErrors

// items without any topics are in the error list
item_list1 must haveSize(2)
item_list1.contains(i1) must equalTo(true)
item_list1.contains(i2) must equalTo(true)

// adding a 'normal' topic does not remove the item from the error list
i1.addTopic(t)
val item_list2 = Item.allWithCatalogErrors
item_list2 must haveSize(2)
item_list2.contains(i1) must equalTo(true)
item_list2.contains(i2) must equalTo(true)

// adding the special 'any' topic removes an item from the error list
i1.addTopic(t_any)
val item_list3 = Item.allWithCatalogErrors
item_list3 must haveSize(1)
item_list3.contains(i1) must equalTo(false)
item_list3.contains(i2) must equalTo(true)

// adding the special 'none' topic removes an item from the error list
i2.addTopic(t_none)
val item_list4 = Item.allWithCatalogErrors
item_list4 must haveSize(0)
item_list4.contains(i1) must equalTo(false)
item_list4.contains(i2) must equalTo(false)
}
}

"#inCollection" in {
running(FakeApplication(additionalConfiguration = inMemoryDatabase())) {
User.create("bob", "bob@example.com", "pass", "roley")
Expand Down
24 changes: 24 additions & 0 deletions test/unit/PublisherSpec.scala
Expand Up @@ -189,6 +189,30 @@ class PublisherSpec extends Specification {
}
}

"#harvests" in {
running(FakeApplication(additionalConfiguration = inMemoryDatabase())) {
val u = User.make("bob", "bob@example.com", "pwd", "role1")
Publisher.all.size must equalTo(0)
val p = Publisher.make(u.id, "pubtag", "pubname", "pubdesc", "pubcat", "pubstatus", Some(""), Some(""))

p.harvests.size must equalTo(0)
val h1 = Harvest.make(p.id, "name", "protocol", "http://www.example.com", "http://example.org", 1, new Date)
p.harvests.size must equalTo(1)
p.harvests.contains(h1) must equalTo(true)
val h2 = Harvest.make(p.id, "name2", "protocol", "http://www.example.com", "http://example.org", 1, new Date)
p.harvests.contains(h1) must equalTo(true)
p.harvests.contains(h2) must equalTo(true)
p.harvests.size must equalTo(2)

val p2 = Publisher.make(u.id, "pubtag2", "pubname", "pubdesc", "pubcat", "pubstatus", Some(""), Some(""))
val h3 = Harvest.make(p2.id, "name3", "protocol", "http://www.example.com", "http://example.org", 1, new Date)
p.harvests.contains(h1) must equalTo(true)
p.harvests.contains(h2) must equalTo(true)
p.harvests.contains(h3) must equalTo(false)
p.harvests.size must equalTo(2)
}
}

"#harvestCount" in {
running(FakeApplication(additionalConfiguration = inMemoryDatabase())) {
val u = User.make("bob", "bob@example.com", "pwd", "role1")
Expand Down

1 comment on commit 1d24701

@richardrodgers
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the gnarliest calls 'Publisher.findById(Collection.findById....' could be rewritten using methods from the cull PR, but it's not worth all the synchronization of PRs to do (can clean up later)
👍

Please sign in to comment.