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.

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

Feature/main/7635 Monograph report #1204

Merged
merged 12 commits into from Oct 19, 2022
Merged

Feature/main/7635 Monograph report #1204

merged 12 commits into from Oct 19, 2022

Conversation

jonasraoni
Copy link
Contributor

No description provided.

@jonasraoni jonasraoni mentioned this pull request Sep 14, 2022
Closed
@jonasraoni jonasraoni force-pushed the feature/main/7635-monograph-report branch 3 times, most recently from 872515f to fd6a4ee Compare September 19, 2022 10:13
bozana
bozana reviewed Sep 19, 2022
View reviewed changes
Copy link
Contributor

@bozana bozana left a comment

Choose a reason for hiding this comment

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

Hi @jonasraoni, it looks good. Just a few comments, mostly questions :-)

plugins/reports/monographReport/MonographReportPlugin.php Outdated Show resolved Hide resolved
plugins/reports/monographReport/MonographReportPlugin.php Outdated Show resolved Hide resolved
plugins/reports/monographReport/MonographReportPlugin.php Show resolved Hide resolved
plugins/reports/monographReport/MonographReportPlugin.php Outdated Show resolved Hide resolved
plugins/reports/monographReport/MonographReportPlugin.php Outdated Show resolved Hide resolved
plugins/reports/monographReport/MonographReportPlugin.php Outdated Show resolved Hide resolved
plugins/reports/monographReport/MonographReportPlugin.php Outdated Show resolved Hide resolved
@bozana
Copy link
Contributor

bozana commented Sep 19, 2022

@jonasraoni, differently to ArticleReport this report does not display all the information about the authors (author ORCID, country, affiliation), also no information about the editors and decisions made, other metadata, and it considers only published submissions. I assume this all is wished/required so?

@jonasraoni
Copy link
Contributor Author

@bozana About what should be included in the report, I'll ask for confirmation... For now, I've just added what was written in the issue :)

@jonasraoni jonasraoni force-pushed the feature/main/7635-monograph-report branch 3 times, most recently from 703b7ad to eb9faa8 Compare September 20, 2022 19:43
@jonasraoni
Copy link
Contributor Author

@bozana About what should be included in the report, I'll ask for confirmation... For now, I've just added what was written in the issue :)

Confirmed! I didn't see the ArticleReport, so I'll probably rebuild this code over it 🤔

@jonasraoni jonasraoni force-pushed the feature/main/7635-monograph-report branch from eb9faa8 to f5a83a3 Compare October 9, 2022 17:57
@jonasraoni
Copy link
Contributor Author

@bozana I've updated this PR yesterday, feel free to review again.
Basically, I've incorporated the data/format from the OJS' ArticleReportPlugin and did some performance tweaks.

Once everything is ok, I'll port the changes to the stable-3_3_0.

bozana
bozana reviewed Oct 12, 2022
View reviewed changes
Copy link
Contributor

@bozana bozana left a comment

Choose a reason for hiding this comment

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

Great work!!! ⭐
Just a few minor comments :-)

plugins/reports/monographReport/MonographReportPlugin.php Outdated Show resolved Hide resolved
plugins/reports/monographReport/MonographReportPlugin.php Outdated Show resolved Hide resolved
plugins/reports/monographReport/MonographReportPlugin.php Outdated Show resolved Hide resolved
plugins/reports/monographReport/locale/en_US/locale.po Outdated Show resolved Hide resolved
@jonasraoni
Copy link
Contributor Author

Thanks @bozana! I'll check what's happening with the build (it works on my MySQL setup), maybe do minor updates, and port it to stable-3_3_0.

@jonasraoni jonasraoni force-pushed the feature/main/7635-monograph-report branch 2 times, most recently from 45890a8 to cd07d17 Compare October 13, 2022 18:56
bozana
bozana reviewed Oct 19, 2022
View reviewed changes
Copy link
Contributor

@bozana bozana left a comment

Choose a reason for hiding this comment

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

@jonasraoni, thanks! Two questions but actually ready to be merged...

plugins/reports/monographReport/Report.php Outdated Show resolved Hide resolved
plugins/reports/monographReport/Report.php Show resolved Hide resolved
@jonasraoni jonasraoni force-pushed the feature/main/7635-monograph-report branch from cd07d17 to 3ea3418 Compare October 19, 2022 13:40
@jonasraoni jonasraoni force-pushed the feature/main/7635-monograph-report branch from 3ea3418 to 74923bc Compare October 19, 2022 13:44
@jonasraoni jonasraoni merged commit 1944a67 into pkp:main Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bozana bozana bozana left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@jonasraoni @bozana