ScienceCommons / api

API for interacting with Curate Science model
http://curatescience.org
MIT License
2 stars 4 forks source link

For recently updated/added articles on homepage, add user name and time of contribution #221

Closed eplebel closed 8 years ago

eplebel commented 9 years ago

Add name of user who updated article and time of update in very small text below each of the Recently Added and Recently Updated articles. Mockup: basic-homepage-enhancements

Notice the user name is bolded and the blue color of our color palette. In the case the contributing user is also an author in the system, the contributing user's name should be hyperlinked to that author's page (e.g., if user "Brent Donnellan" updated an article, clicking on his hyperlinked user name should bring you to that user's author page, i.e., https://www.curatescience.org/beta/#/authors/54). In the case a contributing user is not yet an author in the system, the user name should not be hyperlinked (though eventually such user names should be hyperlinked to their user profile page; this isn't yet possible because user profile pages aren't yet publicly exposed).

alexkyllo commented 9 years ago

Pull requests submitted, check it out at https://curatescience-staging.herokuapp.com/beta/#/ and please let me know if any issues, or merge and close if it looks good.

alexkyllo commented 9 years ago

Just FYI your User record wasn't associated to your Author record, so I shelled into the heroku console and associated them manually. I'm not sure how Users and Authors get associated in the app--I'm not seeing any code that appears to do that...

eplebel commented 9 years ago

Everything looks good, except I noticed that for a few updates there is no username at all, for e.g. (must click "Load More" a few times to see these): issue I can't figure out what's different from these updates/articles to the other ones.

eplebel commented 9 years ago

And I believe you're correct that there's currently no way in the UI for users to link their user account to an author.

I guess this feature should eventually be added in the UI on the author page and/or on the user profile page (I'll soon add this as a new issue)

alexkyllo commented 9 years ago

Yeah, there are a bunch of Article records whose owner_id is null (nil in ruby), meaning they're not associated to any user. Maybe they were created by your web scraper instead of by a human user?

screenshot from 2015-08-16 21 20 31

eplebel commented 9 years ago

Oh right, that's probably it. OK, well that's another minor issue to log for another day. Thanks so much, I'm closing this now!

alexkyllo commented 9 years ago

Yeah if you wanted me to mass-update all the orphan articles to make you the owner, that wouldn't be hard. But at least for now I coded it to look like "Updated a month ago" for those, instead of "Updated by -- a month ago" which wouldn't look as good.

eplebel commented 9 years ago

Great that's awesome, thanks a lot! And yes, please go ahead an mass-update all of the orphan articles to make me the owner, thanks!

eplebel commented 9 years ago

A user (asc12345ension@gmail.com, Alex Aarts) just updated this article (https://www.curatescience.org/beta/#/articles/335234) and I noticed that on the homepage (https://www.curatescience.org/beta/#/), it's incorrectly giving me the credit for doing the update. I assume this is because it's pulling in the name of the creator of the article (myself) rather than the user who made the updates??

alexkyllo commented 9 years ago

Do you know what he changed on the article? This is a little strange, there is only one ModelUpdate for this Article in the database and it has you as the submitter. So it seems like this is caused by an issue with how the updates are being recorded rather than how they are being displayed. Not every change causes a ModelUpdate to be created, it depends on whether it's an update to the article itself, or a linked study, or a comment etc.

eplebel commented 9 years ago

Ya I believe he only added two replications from this article (https://www.curatescience.org/beta/#/articles/215234) to studies in this article (https://www.curatescience.org/beta/#/articles/335234).

Basically model updates should be recorded if a user:

  1. adds a study
  2. adds a replication
  3. adds links to data, materials, or pre-registration
  4. adds or updates study information (IV, DV, sample size, power, effect size)
  5. adds comments

To my understanding, I thought Stephen actually had implemented these in Ruby, but I'm not 100%.

alexkyllo commented 9 years ago

OK I'll check to see what, if any, ModelUpdate objects get created when adding a replication to a study currently.

alexkyllo commented 9 years ago

Adding a comment, study, or replication updates the parent article's updated_at date but does not create a ModelUpdate record. There are no ModelUpdates at the Comment, Replication, or Link level. The only models that record ModelUpdates are Author, Article and Study.

The API is sending the Article model's updated_at date which will be updated anytime a child study or replication is updated, because of the touch: true lines in the Study and Replication models: https://github.com/ScienceCommons/api/blob/master/app/models/study.rb#L12 https://github.com/ScienceCommons/api/blob/master/app/models/replication.rb#L7 It's also updating the Article model's updated_at date when a Comment is added because this line is updating the Article's comment_count: https://github.com/ScienceCommons/api/blob/master/app/models/comment.rb#L22

So the landing page is showing the time of the last update to any child comment, study or replication under the article, but it's showing the name of the user who submitted the last ModelUpdate for the Article itself. That's why it's showing your name even though another user added a replication.

We could start recording ModelUpdate records for Comment, Replication and Link, and query for the name of the person who last submitted a ModelUpdate for any Study, Author, Comment, Replication or Link associated with the Article. I am worried about the performance of this query though, since the landing page already loads pretty slow.

eplebel commented 9 years ago

Interesting, thanks for looking into this in depth. Your proposed solution sounds good to me. In terms of performance, I was thinking we could combine the Recently Added and Recently Updated into one column (called "Recently Added/Updated") and hence we'd only be loading 10 articles instead of 20. I assume that would help, right?

alexkyllo commented 8 years ago

Deployed to staging and PRs opened. Try it out in staging and let me know if you find any issues. Because I had to add a new column to the Article table to track last updater's user ID (any other way would have made the query much too slow), there's no way to populate the updated_by names retroactively, except to mass-update them to your name, which I haven't done yet. For now, it will just say "Updated by Anonymous" for all past updates.

eplebel commented 8 years ago

great, everything looks good!