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

make logo for orcid #49

Closed

rija opened this issue Jan 10, 2018 · 0 comments

Milestone

GigaDB-CCQ-affili...

Owner

rija commented Jan 10, 2018

No description provided.

rija added this to the GigaDB-CCQ-affiliate-login-64 milestone

rija added the in progress label

rija closed this as completed

rija removed the in progress label

rija pushed a commit that referenced this issue


          first draft of Behat scenarios for the acceptance tests for issue #49

e5b9c1a

rija pushed a commit that referenced this issue


          merging two identical authors (#49)

7cf17e4

Second draft of acceptance tests for this feature. Added error paths, removed scenario that did test new path,  and added scenario about bi-directionality of linking.
Will send this one to Chris for review.

rija pushed a commit that referenced this issue


          improved the user story for #49 to remove limitation to two authors

a332f26

rija pushed a commit that referenced this issue


          merging identical authors (#49): updated the database schema and mode…

6726f17

…l classes to support the functionality

rija pushed a commit that referenced this issue


          in test scenarios for issue #49, the dialog box showing authors to me…

09d3d75

…rge now show the detailed names and ORCID id

rija pushed a commit that referenced this issue


          after uncovering the graph nature of the requirements for issue #49, …

96d2342

…I've added additional scenarios to cover the inferred linked authors, the unmerge action becoming the removal of an author from the graph.

rija pushed a commit that referenced this issue


          after uncovering the graph nature of the requirements for issue #49, …

582f4b7

…I've added additional scenarios to cover the inferred linked authors, the unmerge action becoming the removal of an author from the graph. Missed one scenario.

rija pushed a commit that referenced this issue


          started work on merging authors (#49)

added a method to the Author.php model class to return an array of the ids of all authors identical to the model

rija pushed a commit that referenced this issue


          in the work to merge identical authors (#49)

a3fa76e

the method for merging authors can now merge one author with a graph of identical authors

rija pushed a commit that referenced this issue


          Early implementation of merging authors (#49):

7f3cb05

Can get graph of identical authors and can merge author into author, author into graph.

rija pushed a commit that referenced this issue


          In the process of merging with an author (#49)

9b1a38f

make sure merge doesn't proceed if the author merging is already in the graph to merge with.

rija pushed a commit that referenced this issue


          In the process of merging authors (#49) we can merge a graph to anoth…

da73909

…er graph.

So if we have an author we want to merge with an other author and both are distinctly connected to other authors
the two sets of identical authors will be merged into one set of identical author.

rija pushed a commit that referenced this issue


          while implementing unmerging of merged authors as part of #49, I had …

751383c

…the following test error:

CDbException: Column name must be either a string or an array.

Which indicates that a database table is missing a primary key. In this case, it was author_rel that didn't have a primary key index set up in postgresql.

rija pushed a commit that referenced this issue


          in the process of implementing authors merging (#49), I have added an…

51d6354

… unMerge method

the unMerge method will remove an author from the graph by removing all the outward edges from itself.

I also stumbled into the following errors when writing tests that create or delete an AuthorRel object in database:

CDbException: CDbCommand failed to execute the SQL statement: SQLSTATE[23505]: Unique violation: 7 ERROR:  duplicate key value violates unique constraint "author_rel_pkey"
DETAIL:  Key (id)=(1) already exists.

This is because the data fixtures for author_rel are loaded before the tests are run and for some reason either CDbTestCase or CdbFixtureManager set db connection are unable to auto-increment from the last id used in the fixture.

I found a workaround from this issue (which is for a different context/framework version but same problem):
yiisoft/yii2#8298

Didn't put it in a setUp function as it makes the other tests fail.

rija pushed a commit that referenced this issue


          first draft of Behat scenarios for the acceptance tests for issue #49

58252bd

rija pushed a commit that referenced this issue


          merging two identical authors (#49)

11f4c05

Second draft of acceptance tests for this feature. Added error paths, removed scenario that did test new path,  and added scenario about bi-directionality of linking.
Will send this one to Chris for review.

rija pushed a commit that referenced this issue


          improved the user story for #49 to remove limitation to two authors

e3a2a10

rija pushed a commit that referenced this issue


          merging identical authors (#49): updated the database schema and mode…

6c9013a

…l classes to support the functionality

rija pushed a commit that referenced this issue


          in test scenarios for issue #49, the dialog box showing authors to me…

6bae9a8

…rge now show the detailed names and ORCID id

rija pushed a commit that referenced this issue


          after uncovering the graph nature of the requirements for issue #49, …

901c72d

…I've added additional scenarios to cover the inferred linked authors, the unmerge action becoming the removal of an author from the graph.

rija pushed a commit that referenced this issue


          after uncovering the graph nature of the requirements for issue #49, …

bc9b91f

…I've added additional scenarios to cover the inferred linked authors, the unmerge action becoming the removal of an author from the graph. Missed one scenario.

rija pushed a commit that referenced this issue


          started work on merging authors (#49)

b6c47e2

added a method to the Author.php model class to return an array of the ids of all authors identical to the model

rija pushed a commit that referenced this issue


          in the work to merge identical authors (#49)

8ad717d

the method for merging authors can now merge one author with a graph of identical authors

rija pushed a commit that referenced this issue


          Early implementation of merging authors (#49):

24a0606

Can get graph of identical authors and can merge author into author, author into graph.

rija pushed a commit that referenced this issue


          In the process of merging with an author (#49)

eaf75ec

make sure merge doesn't proceed if the author merging is already in the graph to merge with.

rija pushed a commit that referenced this issue


          In the process of merging authors (#49) we can merge a graph to anoth…

00ea7d3

…er graph.

So if we have an author we want to merge with an other author and both are distinctly connected to other authors
the two sets of identical authors will be merged into one set of identical author.

rija pushed a commit that referenced this issue


          while implementing unmerging of merged authors as part of #49, I had …

b9d62ae

…the following test error:

CDbException: Column name must be either a string or an array.

Which indicates that a database table is missing a primary key. In this case, it was author_rel that didn't have a primary key index set up in postgresql.

rija pushed a commit that referenced this issue


          in the process of implementing authors merging (#49), I have added an…

32b2584

… unMerge method

the unMerge method will remove an author from the graph by removing all the outward edges from itself.

I also stumbled into the following errors when writing tests that create or delete an AuthorRel object in database:

CDbException: CDbCommand failed to execute the SQL statement: SQLSTATE[23505]: Unique violation: 7 ERROR:  duplicate key value violates unique constraint "author_rel_pkey"
DETAIL:  Key (id)=(1) already exists.

This is because the data fixtures for author_rel are loaded before the tests are run and for some reason either CDbTestCase or CdbFixtureManager set db connection are unable to auto-increment from the last id used in the fixture.

I found a workaround from this issue (which is for a different context/framework version but same problem):
yiisoft/yii2#8298

Didn't put it in a setUp function as it makes the other tests fail.

rija pushed a commit that referenced this issue


          minor tweaks to the test infrastructure before starting implementing …

8f233dd

…Behat test scenarios for issue #49

rija pushed a commit that referenced this issue


          added a button to the admin author form to initiate process of mergin…

0a85cc2

…g authors (#49)

also added the corresponding target action in the controller.

rija pushed a commit that referenced this issue


          Creating the dialog box for merging two authors (#49)

559cb93

Created a Bootstrap modal (whose dimension can be better controlled than Jquery-UI/CJuiDialog dialog) for the pop up box
Displayed the details of the two author to merge in the pop up was tricky as the details for the the author to merge with
is only known by Javascript, php doesn't have access to it yet, while the details for the author we want to merge come from php
I used the 'rowHtmlOptionsExpression' of CGridView to inline a an author's details as data-attributes to the <tr>
When a row is clicked, the open_control javascript that manage the dialog copies the data attributes to the comparison table.

To add the server side functionality fo merging the authors, I added an mergeAuthors action in the AdminAuthorController
that calls a merging method on the Author model class.

The base url is passed on to Javascript code using registerScript at the top of the admin page. It's cleaner than having all javascript
functions to do their own thing. At the moment only merge_authors() javascript functions use it. but will refactor the other function later.

upon merging success, a flash message is displayed on the admin author view.

rija pushed a commit that referenced this issue


          when merging author together (#49), a relationship is created between…

48b0bff

… them to mark them as being the same author.

There is already an "IsIdenticalTo" relationship in the database I can use, so no need to create a new one.

rija pushed a commit that referenced this issue


          Fix a Behat step definition for verifiying the text on the author mer…

…ging dialog box (#49)

because we use a Bootstrap modal that's invisible by default, the text we won't to match is not seen by Behat, unless we toggle the modal
by injecting javascript into the Webdriver browser.

rija pushed a commit that referenced this issue


          Implemented the abort functionality on the author merging dialog box (#…

e2d412d

…49)

rija pushed a commit that referenced this issue


          Work in progress: working on an implementation of the graph warning t…

5d19c37

…hat tells the admin when the author merging (#49) will actually merge two graphs.

rija pushed a commit that referenced this issue


          An Author model class can return the display names of identical autho…

637db39

…rs (#49)

This will be used in the web frontend on the author merging dialog to show user existing graph(s) before he/she proceed with the merging.

rija pushed a commit that referenced this issue


          in the dialog box for merging two authors (#49)

d8af28f

in the comparison table showing the details of the two authors, the admin can now see the graphs of
identical authors they each has already been merged with.

This required a new action in adminAuthor controller to return through ajax request the display names of the graph for the target author if it exists.

rija pushed a commit that referenced this issue


          in the process of merging two author (#49)

fa03e9e

forbid merging same db record with self

rija pushed a commit that referenced this issue


          for the author merging functionality (#49), implemented some edge cases

f88a11c

like merging authors already in the same graph.

rija pushed a commit that referenced this issue


          for the authors merging (#49) functionality, now display the graphs o…

ed3cbc3

…f identical authors

on the author view and on edit form, if an author is in a graph of identical authors, the graph is displayed with details of each authors.

rija pushed a commit that referenced this issue


          implemented visible feature of unmerging an author from a graph of id…

fd052f4

…entical authors (#49)

rija pushed a commit that referenced this issue


          cosmetic improvements to the status message in the author merging dia…

84ec6fa

…log box (#49)

rija pushed a commit that referenced this issue


          integrating the work done for #49 (author merging) with the work done…

7aa391f

… for #60 (showing datasets of linked author on user profile)

Added a new method to the User model class to access the linked author.
Fixed the schema migration for the author_rel table for syntax error.
Updated the production like database dump (production_like.pgdmp) use for acceptance testing #60  with author_rel
Update UserController to fetch the linked author, and then its graph of identical authors and collect datasets id for all of them

rija pushed a commit that referenced this issue


          integrating the work done for #49 (author merging) with the work done…

7581e39

… for #60 (showing datasets of linked author on user profile)

ensure there is no workflow conflict between merging authors and linking an author to an user (when on the admin author table)

rija pushed a commit that referenced this issue


          minor tweak to enable a spelled Close button on the dialog box for me…

f442a50

…rging two authors (#49)

rija pushed a commit that referenced this issue


          in the authors merging functionality (#49), on an author edit form fi…

4f4b868

…xed bug

the bug was that the Unmerge button shows even when there is no identical authors for this author

rija pushed a commit that referenced this issue


          update the dialog box for linking a user to an author (#56) to show

f3f12dc

the same details about the selected author than in the dialog box for merging two authors (#49)

rija pushed a commit that referenced this issue


          Fixed couple minor Javascript issues in the dialog box for merging tw…

d55a0eb

…o authors (#49) and for attaching user to author (#56)

declare js variables and set to null before using php to conditionally (so sometimes we get undef) assign values to them
replace js includes instruction with indexOf which is more backward compatible (the includes instruction crashes with phantomjs)
removed a redundant var keywords in open_controls function.

rija pushed a commit that referenced this issue


          Removed a non-consequential checks from a background scenario in feat…

a842137

…ure for issue #81 that keeps crashing since adding the feature for #49

rija pushed a commit that referenced this issue


          move out invariant instructions out of the loop (#49)

d8ecd69

rija pushed a commit that referenced this issue


          Merge pull request #176 from rija/merge-authors-49

114eb34

Work related to Author workflow( #49, #56, #57, and #60)

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