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.

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

workflows: add manual_merge workflow #3005

Merged
merged 3 commits into from
Nov 29, 2017
Merged

workflows: add manual_merge workflow #3005

merged 3 commits into from
Nov 29, 2017

Conversation

jacquerie
Copy link
Contributor

@jacquerie jacquerie commented Nov 28, 2017

Related Issue

Supersedes #2904.

Checklist:

  • I have all the information that I need (if not, move to RFC and look for it).
  • I linked the related issue(s) in the corresponding commit logs.
  • I wrote good commit log messages.
  • My code follows the code style of this project.
  • I've added any new docs if API/utils methods were added.
  • I have updated the existing documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@ghost ghost assigned jacquerie Nov 28, 2017
@ghost ghost added the Status: WIP label Nov 28, 2017
update['new_record'] = get_record_ref(
head['control_number'],
endpoint='record'
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not right, as it would generate a JSON reference like

{"$ref": "http://localhost:5000/api/record/4328"}

which isn't correct, because we don't redirect routes like /api/record/<int:control_number>, just /record/<int:control_number>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As other parts of the code assume that we're merging Literature records (most notably, we don't have configuration otherwise in inspire-json-merger) it's ok to hardcode it here too.

from inspirehep.modules.workflows.models import WorkflowsRecordSources


def get_head_source(head_uuid):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This function is in a dedicated tasks module, but isn't a task... It should probably be moved to utils.



def test_get_head_source_return_arxiv_when_one_arxive_source_present(app, simple_record):
rec = record_insert_or_replace(simple_record)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test doesn't clean after itself: it creates this record, but then it doesn't delete it.

json_update = fake_record('While this is the update', 2)

head = record_insert_or_replace(json_head)
update = record_insert_or_replace(json_update)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

zzacharo and others added 3 commits November 28, 2017 21:58
Signed-off-by: Antonio Cesarano <[email protected]>
Co-authored-by: Riccardo Candido <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants