Skip to content

Conversation

Hafsa-Naeem
Copy link
Contributor

@Hafsa-Naeem Hafsa-Naeem commented Sep 3, 2025

fixes #10962

Copy link
Member

@touhidurabir touhidurabir left a comment

Choose a reason for hiding this comment

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

@Hafsa-Naeem I have added few comments . lets port these for OMP/OPS and we can finalise the PR to merge after one final review .

{
public function up(): void
{
DB::transaction(function () {
Copy link
Member

Choose a reason for hiding this comment

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

lets not use DB transaction until we reach to resolution on #10595 . I personally love to have this approach but as we haven't started to use this consistently .

Comment on lines 26 to 31
$map = [
'journalAcronym' => 'contextAcronym',
'journalName' => 'contextName',
'journalUrl' => 'contextUrl',
'journalSignature' => 'contextSignature',
];
Copy link
Member

Choose a reason for hiding this comment

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

If I understand correctly, this is only considering the case of OJS but not OMP/OPS where we have like pressName or serverName , e.g specific to OMP/OPS . I think this is good but this core/shared lib class should have only the core migration define and abstract method for variable mapping so that each app can have it's own variable modifications . As by updating the classes/migration/upgrade/v3_4_0/InstallEmailTemplates.php, it will have effect when updating form 3.3.4-x but for installation upgrading form 3.5.0-x (already upgraded to 3.5.0), need to accommodate that also . Please correct me if I am mistaken .

@Hafsa-Naeem Hafsa-Naeem force-pushed the i10962-stable-3_5_0-fix branch from 4fdf46d to c29e2f5 Compare October 16, 2025 21:11
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.

2 participants