Skip to content

do not revert on current revision #5564

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
Closed
wants to merge 1 commit into from
Closed

do not revert on current revision #5564

wants to merge 1 commit into from

Conversation

CleverFool77
Copy link
Member

@CleverFool77 CleverFool77 commented Apr 21, 2019 •
edited
Loading

Fixes #5551 (<=== Add issue number here)

Description

Earlier I tried removing the revert button for current revision but the ui became inconsistent because of this.
So Instead I made revert button disabled while showing the information on tooltip that already on current revision and can't revert anymore.

Screenshot from 2019-04-21 16-43-14


  • PR is descriptively titled 📑 and links the original issue above 🔗
  • tests pass -- look for a green checkbox ✔️ a few minutes after opening your PR -- or run tests locally with rake test
  • code is in uniquely-named feature branch and has no merge conflicts 📁
  • screenshots/GIFs are attached 📎 in case of UI updation
  • ask @publiclab/reviewers for help, in a comment below

Thanks!

@plotsbot
Copy link
Collaborator

2 Messages
📖 @CleverFool77 Thank you for your pull request! I’m here to help with some tips and recommendations. Please take a look at the list provided and help us review and accept your contribution! And don’t be discouraged if you see errors – we’re here to help.
📖 This pull request doesn’t link to a issue number. Please refer to the issue it fixes (if any) in the body of your PR, in the format: Fixes #123.

Generated by 🚫 Danger

grvsachdeva
grvsachdeva requested changes Apr 28, 2019
View reviewed changes
Copy link
Member

@grvsachdeva grvsachdeva left a comment

Choose a reason for hiding this comment

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

Hi @CleverFool77, you handled the case nicely on frontend but still one can generate redundant revisions by making get request so let's handle this case in backend too. You will be doing changes for that in wiki_controller#revert. We can also add a test for it.

What do you think?

Thanks!

@CleverFool77 CleverFool77 reopened this Sep 4, 2019
@CleverFool77
Copy link
Member Author

Hi @gauravano I went through this, In backend, we are just directly replacing the new revision with the all old duplicate revision.
Even the timestamp of new_rev.timestamp = DateTime.now.to_i is when the change was done. There is nowhere stored ,any kind of flag that this revision is current.
And Till the time, we click on revert even after editing the latest change few seconds ago. The seconds must have already passed. So we can't check it using timestamp.
Or Should new flag be added ?
cc: @jywarren

@grvsachdeva
Copy link
Member

hey @CleverFool77,

Here's how you can prevent the request in backend:

First, let's see what we have in our tables:

[24] pry(main)> Node.last
  Node Load (0.2ms)  SELECT  "node".* FROM "node" ORDER BY "node"."nid" DESC LIMIT ?  [["LIMIT", 1]]
=> #<Node:0x00007f897835a0a8
 nid: 44,
 vid: 46,
 type: "page",
 language: "",
 title: "test page",
 uid: 1,
[25] pry(main)> Node.last.revisions
  Node Load (0.2ms)  SELECT  "node".* FROM "node" ORDER BY "node"."nid" DESC LIMIT ?  [["LIMIT", 1]]
  Revision Load (0.2ms)  SELECT "node_revisions".* FROM "node_revisions" WHERE "node_revisions"."nid" = ? ORDER BY "node_revisions"."timestamp" DESC  [["nid", 44]]
=> [#<Revision:0x00007f8982cb1cf0 vid: 47, nid: 44, uid: 1, title: "test page", body: "something meaningfull!!!\r\n\r\none\r\n\r\ntwo", teaser: "", log: "", timestamp: 1567891987, format: 1, status: 1>,
 #<Revision:0x00007f8982cb1ae8 vid: 46, nid: 44, uid: 1, title: "test page", body: "something meaningfull!!!\r\n\r\none\r\n\r\ntwo", teaser: "", log: "", timestamp: 1567891936, format: 1, status: 1>,
 #<Revision:0x00007f8982cb18e0 vid: 45, nid: 44, uid: 1, title: "test page", body: "something meaningfull!!!\r\n\r\none", teaser: "", log: "", timestamp: 1567891904, format: 1, status: 1>,
 #<Revision:0x00007f8982cb16b0 vid: 44, nid: 44, uid: 1, title: "test page", body: "something meaningfull!!!", teaser: "", log: "", timestamp: 1567891839, format: 1, status: 1>]
[26] pry(main)>

The above is a snippet of rails console. The revisions table store revisions of each node. If we perform a query like node.revisions.last then it will give us the latest revision which is in effect at that time. So, we can compare the id which we receive in param with this last revision Id and if they're same we'll not revert.

(The vid mentioned on each node doesn't point to the lastest revision. It points to the revision which was last created without revert action)

@CleverFool77
Copy link
Member Author

CleverFool77 commented Sep 8, 2019 via email

@SidharthBansal
Copy link
Member

There are some conflicts. Kindly rebase

@SidharthBansal
Copy link
Member

As the person is inactive for more than a month, I am closing the PR. In case you want to push changes please feel free to open a new PR OR reopen this PR and add additional changes to it.
Thanks for contributing on Public Lab

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

@grvsachdeva grvsachdeva Awaiting requested review from grvsachdeva

@jywarren jywarren Awaiting requested review from jywarren

Assignees
No one assigned
Labels
in progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Revert on current revision history
4 participants
@CleverFool77 @plotsbot @grvsachdeva @SidharthBansal