Skip to content

feat: library release management - library resolver - #101

Open
Peterburnett wants to merge 2 commits into
devfrom
library-release-phase-0
Open

Peterburnett wants to merge 2 commits into
devfrom
library-release-phase-0

Conversation

@Peterburnett

Copy link
Copy Markdown

No description provided.

Comment thread classes/library_release_artifact_storage.php Outdated
Comment thread classes/library_release_artifact_storage.php Outdated
Comment thread classes/library_release_artifact_storage.php Outdated
Comment thread classes/library_release_artifact_storage.php Outdated
Comment thread classes/library_release_artifact_storage.php Outdated
Comment thread classes/library_release_resolver.php Outdated
Comment thread classes/library_release_resolver.php Outdated
Comment thread classes/library_release_resolver.php Outdated
@Peterburnett
Peterburnett force-pushed the library-release-phase-0 branch from c4cf096 to fa2ee00 Compare August 26, 2026 01:16
Comment thread classes/library_release_resolver.php
Comment thread db/install.xml Outdated
Comment thread db/upgrade.php Outdated
Comment thread db/upgrade.php Outdated
@Peterburnett
Peterburnett force-pushed the library-release-phase-0 branch from 961757b to 4744f53 Compare August 27, 2026 01:18

@rhell4 rhell4 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

My biggest concern is recreating a few of the existing tables with a few field differences to accommodate the new faction and release relations.

We should be able to do this by changing the existing tables. When the release management feature is deployed there should be no need to keep these tables unchanged as they would no longer be used the original way.

Comment thread db/install.php
function xmldb_hvp_install() {
global $DB;

if (!$DB->record_exists('hvp_library_release_state', ['id' => 1])) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

While an id of 1 is almost always correct it isn't always guaranteed. We should just be checking if any record exists at all regardless of id.

Comment thread db/install.xml
Comment on lines +378 to +394
<TABLE NAME="hvp_library_release_content_libraries" COMMENT="Artifact dependencies selected for HVP activities">
<FIELDS>
<FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/>
<FIELD NAME="hvp_id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/>
<FIELD NAME="artifact_id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/>
<FIELD NAME="dependency_type" TYPE="char" LENGTH="20" NOTNULL="true" SEQUENCE="false"/>
<FIELD NAME="drop_css" TYPE="int" LENGTH="1" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/>
<FIELD NAME="weight" TYPE="int" LENGTH="10" NOTNULL="true" DEFAULT="0" SEQUENCE="false"/>
</FIELDS>
<KEYS>
<KEY NAME="primary" TYPE="primary" FIELDS="id"/>
</KEYS>
<INDEXES>
<INDEX NAME="content_artifact_type" UNIQUE="true" FIELDS="hvp_id, artifact_id, dependency_type"/>
<INDEX NAME="hvp_id" UNIQUE="false" FIELDS="hvp_id"/>
</INDEXES>
</TABLE>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We shouldn't need to create a whole new table for this, we can add the new artifact_id field and remove the old library_id from the existing hvp_contents_libraries table.

Comment thread db/install.xml
Comment on lines +332 to +346
<TABLE NAME="hvp_library_artifact_dependencies" COMMENT="Dependencies between immutable library artifacts">
<FIELDS>
<FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/>
<FIELD NAME="artifact_id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/>
<FIELD NAME="required_artifact_id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/>
<FIELD NAME="dependency_type" TYPE="char" LENGTH="20" NOTNULL="true" SEQUENCE="false"/>
</FIELDS>
<KEYS>
<KEY NAME="primary" TYPE="primary" FIELDS="id"/>
</KEYS>
<INDEXES>
<INDEX NAME="artifact_required_type" UNIQUE="true" FIELDS="artifact_id, required_artifact_id, dependency_type"/>
<INDEX NAME="required_artifact_id" UNIQUE="false" FIELDS="required_artifact_id"/>
</INDEXES>
</TABLE>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We shouldn't need to create a whole new table for this, we can add the new artifact_id and required_artifcat_id fields to hvp_libraries_libraries and remove the library_id and required_library_id fields.

Comment thread db/install.xml
Comment on lines +347 to +360
<TABLE NAME="hvp_library_artifact_languages" COMMENT="Translations for immutable library artifacts">
<FIELDS>
<FIELD NAME="id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="true"/>
<FIELD NAME="artifact_id" TYPE="int" LENGTH="10" NOTNULL="true" SEQUENCE="false"/>
<FIELD NAME="language_code" TYPE="char" LENGTH="31" NOTNULL="true" SEQUENCE="false"/>
<FIELD NAME="language_json" TYPE="text" NOTNULL="true" SEQUENCE="false"/>
</FIELDS>
<KEYS>
<KEY NAME="primary" TYPE="primary" FIELDS="id"/>
</KEYS>
<INDEXES>
<INDEX NAME="artifact_language" UNIQUE="true" FIELDS="artifact_id, language_code"/>
</INDEXES>
</TABLE>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We shouldn't need to create a whole new table for this, we should be able to convert the library_id field to artifact_id in the existing hvp_libraries_languages table.


$DB->delete_records('hvp_library_release_state');
hvp_upgrade_2026082600();
hvp_upgrade_2026082600();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This shouldn't need to be called twice.

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