pkp/pkp-lib#10157 Move issueIds out of publication_settings #4396
Conversation
|
|
Hafsa-Naeem
force-pushed
the
move_issue_id
branch
from
c7b8b27 to
87d8bdd
Compare
Hafsa-Naeem
changed the title
Hafsa-Naeem
force-pushed
the
move_issue_id
branch
from
87d8bdd to
7cfb8f3
Compare
There was a problem hiding this comment.
@Hafsa-Naeem Besides the comments, you'll need to create a migration in the classes/migration/upgrade/v3_5_0 folder to:
- Create the field for users which are upgrading from OJS < 3.5
- The field should be filled with the content from the
publication_settings - The data from the
publication_settingsmust be cleared.
Take a look at the file /dbscripts/xml/upgrade.xml.
| @@ -242,6 +242,10 @@ public function up(): void | |||
| $table->foreign('doi_id')->references('doi_id')->on('dois')->nullOnDelete(); | |||
| $table->index(['doi_id'], 'publications_doi_id'); | |||
|
|
|||
| $table->bigInteger('issue_id')->nullable(); | |||
| $table->foreign('issue_id')->references('issue_id')->on('issues')->onDelete('set null'); | |||
There was a problem hiding this comment.
| $table->foreign('issue_id')->references('issue_id')->on('issues')->onDelete('set null'); | |
| $table->foreign('issue_id')->references('issue_id')->on('issues')->nullOnDelete(); |
| JOIN publication_settings psissue ON (psissue.publication_id = p.publication_id AND psissue.setting_name='issueId' AND psissue.locale='') | ||
| JOIN issues i ON (CAST(i.issue_id AS CHAR(20)) = psissue.setting_value) | ||
| WHERE i.published = 1 AND j.enabled = 1 AND p.status = 3" | ||
| JOIN issues i ON (i.issue_id = p.issue_id) |
There was a problem hiding this comment.
This PR is for the 3.5, you don't need to touch older upgrade scripts. At this point your new field will not exist yet.
| @@ -90,9 +90,23 @@ protected function buildOrphanedEntityProcessor(): void | |||
| } | |||
| $affectedRows += $this->deleteOptionalReference('publications', 'section_id', 'sections', 'section_id'); | |||
| // Remaining cleanups are inherited | |||
| $rows = DB::table('publications AS p') | |||
There was a problem hiding this comment.
The same as above, you don't need to touch the upgrade scripts for versions < 3.5.
classes/search/ArticleSearchDAO.php
Outdated
| @@ -92,7 +92,7 @@ public function getPhraseResults($journal, $phrase, $publishedFrom = null, $publ | |||
| FROM | |||
| submissions s | |||
| JOIN publications p ON (p.publication_id = s.current_publication_id) | |||
| JOIN publication_settings ps ON (ps.publication_id = p.publication_id AND ps.setting_name=\'issueId\' AND ps.locale=\'\') | |||
| JOIN issues i ON (i.issue_id = p.issue_id AND i.journal_id = s.context_id) | |||
There was a problem hiding this comment.
You forgot to remove the line below. Also, I think we can trust enough in the code to drop the AND i.journal_id = s.context_id
| $join->where('psissue.setting_name', '=', DB::raw('\'issueId\'')); | ||
| $join->where('psissue.locale', '=', DB::raw('\'\'')); | ||
| }) | ||
| ->join('issues AS i', 'i.issue_id', '=', 'p.issue_id') |
There was a problem hiding this comment.
You forgot to remove the line below.
| $q->joinSub($issueSubmissionIds, 'is', function ($join) { | ||
| $join->on('metrics_submission_geo_monthly.' . PKPStatisticsHelper::STATISTICS_DIMENSION_SUBMISSION_ID, '=', 'is.submission_id'); | ||
| }); |
There was a problem hiding this comment.
Hmm, I think this join (also appear at the next file (classes/services/queryBuilders/StatsPublicationQueryBuilder.php), would be better written with a whereExists(), but as it's not part of your task, you might ignore.
classes/submission/Collector.php
Outdated
| $q->whereIn('s.submission_id', function ($query) { | ||
| $query->select('issue_p.submission_id') | ||
| ->from('publications AS issue_p') | ||
| ->join('publication_settings as issue_ps', 'issue_p.publication_id', '=', 'issue_ps.publication_id') | ||
| ->where('issue_ps.setting_name', '=', 'issueId') | ||
| ->whereIn('issue_ps.setting_value', array_map('strval', $this->issueIds)); | ||
| ->whereIn('issue_p.issue_id', $this->issueIds); | ||
| }); |
There was a problem hiding this comment.
The parent class has a join (i.e. publications AS po) that you might use, and it will greatly simplify these lines of code.
Hafsa-Naeem
force-pushed
the
move_issue_id
branch
3 times, most recently
from
b61798a to
7cfb8f3
Compare
Hafsa-Naeem
force-pushed
the
move_issue_id
branch
3 times, most recently
from
16bbd3b to
9e96b8f
Compare
Hafsa-Naeem
force-pushed
the
move_issue_id
branch
from
9e96b8f to
493aaf8
Compare
fix #10157