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

[publication] Add columns to publications #7361

Merged
merged 14 commits into from
Oct 14, 2022

Conversation

pierre-p-s
Copy link
Contributor

@pierre-p-s pierre-p-s commented Mar 1, 2021

Brief summary of changes

  • Added 3 data columns to the publication module:
    - Project,
    - Date published,
    - Journal,
    - link and
    - publishing status (dropdown menu with: in progress or published)

Testing instructions (if applicable)

  1. In publication module, propose a project entering data in the new columns
  2. After the project is proposed, go into the browse tab and click on the project. Check that the new columns appear and that the data matches the one you just entered

Link(s) to related issue(s)

@pierre-p-s pierre-p-s added the "Help! I don't understand Travis!" PR is having a beef with TRAVIS. Someone needs to help label Mar 1, 2021
@kongtiaowang kongtiaowang added State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) and removed State: Needs rebase PR that needs to be rebased to proceed (conflicts, wrong branch...) labels Mar 1, 2021
@kongtiaowang
Copy link
Contributor

rebase after #7369

@pierre-p-s
Copy link
Contributor Author

rebase after #7369

Thanks.

@kongtiaowang
Copy link
Contributor

kongtiaowang commented Mar 10, 2021

The schema SQL file and RB_file need to be modified as well.

CREATE TABLE `publication` (

@pierre-p-s pierre-p-s force-pushed the 2021_03_01_Publication_Add_Columns branch from f7f23cd to 649300f Compare March 10, 2021 19:45
@pierre-p-s
Copy link
Contributor Author

The schema SQL file and RB_file need to be modified as well.

CREATE TABLE `publication` (

I rebased and added the changes to the schema. Not sure what I should do about the RB_Files

@pierre-p-s pierre-p-s removed the "Help! I don't understand Travis!" PR is having a beef with TRAVIS. Someone needs to help label Mar 10, 2021
@driusan
Copy link
Collaborator

driusan commented Mar 10, 2021

This should be going to the main branch

@pierre-p-s pierre-p-s force-pushed the 2021_03_01_Publication_Add_Columns branch from e327977 to 5913c9f Compare March 11, 2021 15:21
@pierre-p-s pierre-p-s changed the base branch from 23.0-release to main March 11, 2021 15:22
@kongtiaowang
Copy link
Contributor

Please ignore the RB files. It doesn't change the schema.

@pierre-p-s
Copy link
Contributor Author

pierre-p-s commented Mar 11, 2021

I cant seem to be able to run make dev on aces/main. I am getting the error attached below. If someone knows how to fix it please let me know.

Screen Shot 2021-03-11 at 10 39 45 AM

@kongtiaowang
Copy link
Contributor

@pierre-p-s try: "make clean" then "make dev", it may solve your issue. Good luck.

@pierre-p-s
Copy link
Contributor Author

@pierre-p-s try: "make clean" then "make dev", it may solve your issue. Good luck.

It did not work for me

@ridz1208
Copy link
Collaborator

@pierre-p-s what versions of node and npm do you have ?

node -v
npm -v

@pierre-p-s
Copy link
Contributor Author

node -v : v12.16.2

npm -v : 6.14.4

Do I need to update them?

@ridz1208
Copy link
Collaborator

Do I need to update them?

I don't think so I have the same versions and it works for me

@driusan
Copy link
Collaborator

driusan commented Mar 19, 2021

have you tried deleting vendor and node_modules before running make dev?

@pierre-p-s
Copy link
Contributor Author

have you tried deleting vendor and node_modules before running make dev?

Yes

Copy link
Contributor

@kongtiaowang kongtiaowang left a comment

Choose a reason for hiding this comment

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

LGTM

@kongtiaowang kongtiaowang added the Passed manual tests PR has been successfully tested by at least one peer label Apr 28, 2021
@pierre-p-s pierre-p-s removed the Passed manual tests PR has been successfully tested by at least one peer label Apr 28, 2021
@pierre-p-s
Copy link
Contributor Author

We found a bug related to this. Fixing it now

@kongtiaowang kongtiaowang added the State: Needs work PR awaiting additional work by the author to proceed label Apr 28, 2021
@driusan
Copy link
Collaborator

driusan commented May 3, 2021

@pierre-p-s is this still needs work or can @kongtiaowang re-review it?

@pierre-p-s
Copy link
Contributor Author

@driusan Sorry forgot to delete the needs work tag. Yes it s ready for review

@pierre-p-s pierre-p-s removed the State: Needs work PR awaiting additional work by the author to proceed label May 3, 2021
@kongtiaowang kongtiaowang added the State: Needs work PR awaiting additional work by the author to proceed label May 8, 2021
@kongtiaowang
Copy link
Contributor

After the 4th commit, the publication module can't save a publication into the database.

@pierre-p-s pierre-p-s removed the State: Needs work PR awaiting additional work by the author to proceed label May 13, 2021
@cmadjar cmadjar added the State: Discussion required PR or issue that requires the resolution of a discussion with the relevant parties to proceed label Jun 8, 2021
SQL/0000-00-00-schema.sql Show resolved Hide resolved
SQL/0000-00-00-schema.sql Outdated Show resolved Hide resolved
@@ -66,9 +70,8 @@ function uploadPublication() : void
$leadInvID = $db->pselectOne(
'SELECT PublicationCollaboratorID '.
'FROM publication_collaborator '.
'WHERE Name = :n OR Email = :e',
'WHERE Email = :e',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should be a different pull request if it's a different feature/bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reverted, will send a new PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pierre-p-s pierre-p-s removed the State: Discussion required PR or issue that requires the resolution of a discussion with the relevant parties to proceed label Jun 10, 2021
@pierre-p-s pierre-p-s closed this Jun 10, 2021
@pierre-p-s pierre-p-s reopened this Jun 10, 2021
modules/publication/jsx/publicationIndex.js Outdated Show resolved Hide resolved
modules/publication/php/publication.class.inc Outdated Show resolved Hide resolved
@pierre-p-s
Copy link
Contributor Author

Addressed comments and added a new field: Project.

@ridz1208
Copy link
Collaborator

ridz1208 commented Oct 5, 2022

@driusan this can be merged if you take a quick look at it

@driusan driusan merged commit 663536d into aces:main Oct 14, 2022
@ridz1208 ridz1208 added this to the 25.0.0 milestone Mar 6, 2023
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.

5 participants