Skip to content

Commit

Permalink
pkp/pkp-lib/#10157 Move issueIds out of publication_settings
Browse files Browse the repository at this point in the history
  • Loading branch information
Hafsa-Naeem committed Aug 12, 2024
1 parent b08b8bb commit 7cfb8f3
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 54 deletions.
4 changes: 4 additions & 0 deletions classes/migration/install/OJSMigration.php
Original file line number Diff line number Diff line change
Expand Up @@ -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');
$table->index(['issue_id'], 'publications_issue_id_index');

$table->index(['url_path'], 'publications_url_path');
});
// The following foreign key relationships are for tables defined in SubmissionsMigration
Expand Down
16 changes: 7 additions & 9 deletions classes/migration/upgrade/v3_4_0/I7901_Duplicate_OAI_IDs.php
Original file line number Diff line number Diff line change
Expand Up @@ -24,26 +24,24 @@ public function up(): void
switch (DB::getDriverName()) {
case 'mysql':
DB::unprepared(
"DELETE dot
'DELETE dot
FROM data_object_tombstones dot
JOIN submissions s ON (dot.data_object_id = s.submission_id)
JOIN journals j ON (j.journal_id = s.context_id)
JOIN publications p ON (s.current_publication_id = p.publication_id)
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)
WHERE i.published = 1 AND j.enabled = 1 AND p.status = 3'
);
break;
case 'pgsql':
DB::unprepared(
"DELETE FROM data_object_tombstones dot
USING submissions s, journals j, publications p, publication_settings psissue, issues i
'DELETE FROM data_object_tombstones dot
USING submissions s, journals j, publications p, issues i
WHERE dot.data_object_id = s.submission_id
AND j.journal_id = s.context_id
AND s.current_publication_id = p.publication_id
AND psissue.publication_id = p.publication_id
AND psissue.setting_name='issueId' AND psissue.locale='' AND (CAST(i.issue_id AS CHAR(20)) = psissue.setting_value)
AND i.published = 1 AND j.enabled = 1 AND p.status = 3"
AND i.issue_id = p.issue_id
AND i.published = 1 AND j.enabled = 1 AND p.status = 3'
);
break;
}
Expand Down
33 changes: 14 additions & 19 deletions classes/migration/upgrade/v3_4_0/PreflightCheckMigration.php
Original file line number Diff line number Diff line change
Expand Up @@ -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')
->leftJoin('issues AS i', 'p.issue_id', '=', 'i.issue_id')
->whereNull('i.issue_id')
->whereNotNull('p.issue_id') // Ensure we're only looking at non-null issue_ids
->select('p.submission_id', 'p.publication_id', 'p.issue_id')
->get();

foreach ($rows as $row) {
$this->_installer->log("The publication ID ({$row->publication_id}) for the submission ID {$row->submission_id} is assigned to an invalid issue ID \"{$row->issue_id}\", its issue_id will be set to NULL.");
$affectedRows += DB::table('publications')
->where('publication_id', '=', $row->publication_id)
->update(['issue_id' => null]); // Reset invalid issue_id to null
}
return $affectedRows;
});


$this->addTableProcessor('publication_galleys', function (): int {
$affectedRows = 0;
// Depends directly on ~3 entities: doi_id->dois.doi_id(not found in previous version) publication_id->publications.publication_id submission_file_id->submission_files.submission_file_id
Expand Down Expand Up @@ -229,25 +243,6 @@ protected function buildOrphanedEntityProcessor(): void
return $affectedRows;
});

// Support for the issueId setting
$this->addTableProcessor('publication_settings', function (): int {
$affectedRows = 0;
$rows = DB::table('publications AS p')
->join('publication_settings AS ps', 'ps.publication_id', '=', 'p.publication_id')
->leftJoin('issues AS i', 'ps.setting_value', '=', DB::raw('CAST(i.issue_id AS CHAR(20))'))
->where('ps.setting_name', 'issueId')
->whereNull('i.issue_id')
->get(['p.submission_id', 'p.publication_id', 'ps.setting_value']);
foreach ($rows as $row) {
$this->_installer->log("The publication ID ({$row->publication_id}) for the submission ID {$row->submission_id} is assigned to an invalid issue ID \"{$row->setting_value}\", its value will be updated to NULL");
$affectedRows += DB::table('publication_settings')
->where('publication_id', '=', $row->publication_id)
->where('setting_name', 'issueId')
->where('setting_value', $row->setting_value)
->delete();
}
return $affectedRows;
});
}

protected function getEntityRelationships(): array
Expand Down
8 changes: 3 additions & 5 deletions classes/oai/ojs/OAIDAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,8 @@ public function &getSection($sectionId)
* @param int $total
*
* @return array OAISet
*
* @hook OAIDAO::getJournalSets [[$this, $journalId, $offset, $limit, $total, &$sets]]
*/
public function &getJournalSets($journalId, $offset, $limit, &$total)
{
Expand Down Expand Up @@ -269,11 +271,7 @@ public function _getRecordsRecordSetQuery($setIds, $from, $until, $set, $submiss
's.section_id AS section_id',
])
->join('publications AS p', 'a.current_publication_id', '=', 'p.publication_id')
->join('publication_settings AS psissue', function ($join) {
$join->on('psissue.publication_id', '=', 'p.publication_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')
->join('issues AS i', DB::raw('CAST(i.issue_id AS CHAR(20))'), '=', 'psissue.setting_value')
->join('sections AS s', 's.section_id', '=', 'p.section_id')
->join('journals AS j', 'j.journal_id', '=', 'a.context_id')
Expand Down
3 changes: 2 additions & 1 deletion classes/publication/DAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ class DAO extends \PKP\publication\DAO
'status' => 'status',
'urlPath' => 'url_path',
'version' => 'version',
'doiId' => 'doi_id'
'doiId' => 'doi_id',
'issueId' => 'issue_id'
];

/**
Expand Down
2 changes: 1 addition & 1 deletion classes/search/ArticleSearchDAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)
JOIN issues i ON (CAST(i.issue_id AS CHAR(20)) = ps.setting_value AND i.journal_id = s.context_id)
JOIN submission_search_objects o ON (s.submission_id = o.submission_id)
JOIN journals j ON j.journal_id = s.context_id
Expand Down
8 changes: 4 additions & 4 deletions classes/services/queryBuilders/StatsGeoQueryBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,11 @@ public function filterByIssues(array $issueIds): self
protected function _getAppSpecificQuery(Builder &$q): void
{
if (!empty($this->issueIds)) {
$issueSubmissionIds = DB::table('publications as p')->select('p.submission_id')->distinct()
->from('publications as p')
->leftJoin('publication_settings as ps', 'ps.setting_name', '=', DB::raw('\'issueId\''))
$issueSubmissionIds = DB::table('publications as p')
->select('p.submission_id')
->distinct()
->where('p.status', Submission::STATUS_PUBLISHED)
->whereIn('ps.setting_value', $this->issueIds);
->whereIn('p.issue_id', $this->issueIds);
$q->joinSub($issueSubmissionIds, 'is', function ($join) {
$join->on('metrics_submission_geo_monthly.' . PKPStatisticsHelper::STATISTICS_DIMENSION_SUBMISSION_ID, '=', 'is.submission_id');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,8 @@ protected function _getAppSpecificQuery(Builder &$q): void
{
if (!empty($this->issueIds)) {
$issueSubmissionIds = DB::table('publications as p')->select('p.submission_id')->distinct()
->from('publications as p')
->leftJoin('publication_settings as ps', 'ps.setting_name', '=', DB::raw('\'issueId\''))
->where('p.status', Submission::STATUS_PUBLISHED)
->whereIn('ps.setting_value', $this->issueIds);
->whereIn('p.issue_id', $this->issueIds);
$q->joinSub($issueSubmissionIds, 'is', function ($join) {
$join->on('metrics_submission.' . PKPStatisticsHelper::STATISTICS_DIMENSION_SUBMISSION_ID, '=', 'is.submission_id');
});
Expand Down
5 changes: 2 additions & 3 deletions classes/submission/Collector.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,7 @@ public function getQueryBuilder(): Builder
$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);
});
}

Expand All @@ -75,6 +73,7 @@ public function getQueryBuilder(): Builder
return $q;
}


/**
* Add APP-specific filtering methods for submission sub objects DOI statuses
*
Expand Down
12 changes: 3 additions & 9 deletions classes/submission/DAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,9 @@ public function getExportable(int $contextId, $pubIdType = null, $title = null,
->leftJoin('publications AS p', 's.current_publication_id', '=', 'p.publication_id')
->leftJoin('publication_settings AS ps', 'p.publication_id', '=', 'ps.publication_id')
->when(
$issueId,
fn (Builder $q) => $q->leftJoin(
'publication_settings AS psi',
fn (JoinClause $j) => $j->on('p.publication_id', '=', 'psi.publication_id')
->where('psi.setting_name', '=', 'issueId')
->where('psi.locale', '=', '')
)
$pubIdType != null,
fn (Builder $q) => $q->leftJoin('publication_settings AS pspidt', 'p.publication_id', '=', 'pspidt.publication_id')
)
->when($pubIdType != null, fn (Builder $q) => $q->leftJoin('publication_settings AS pspidt', 'p.publication_id', '=', 'pspidt.publication_id'))
->when($title != null, fn (Builder $q) => $q->leftJoin('publication_settings AS pst', 'p.publication_id', '=', 'pst.publication_id'))
->when(
$author != null,
Expand Down Expand Up @@ -89,7 +83,7 @@ public function getExportable(int $contextId, $pubIdType = null, $title = null,
->when($pubIdType != null, fn (Builder $q) => $q->where('pspidt.setting_name', '=', "pub-id::{$pubIdType}")->whereNotNull('pspidt.setting_value'))
->when($title != null, fn (Builder $q) => $q->where('pst.setting_name', '=', 'title')->where('pst.setting_value', 'LIKE', "%{$title}%"))
->when($author != null, fn (Builder $q) => $q->whereRaw("CONCAT(COALESCE(asgs.setting_value, ''), ' ', COALESCE(asfs.setting_value, '')) LIKE ?", [$author]))
->when($issueId != null, fn (Builder $q) => $q->where('psi.setting_value', '=', $issueId))
->when($issueId != null, fn (Builder $q) => $q->where('p.issue_id', '=', $issueId))
->when(
$pubIdSettingName,
fn (Builder $q) => $q->when(
Expand Down

0 comments on commit 7cfb8f3

Please sign in to comment.