Skip to content

Nde 95 create GitHub actions to run build script for nde customization package and addons#55

Merged
adamshire123 merged 12 commits into
mit-mainfrom
NDE-95-create-github-actions-to-run-build-script-for-nde-customization-package-and-addons
May 18, 2026
Merged

Nde 95 create GitHub actions to run build script for nde customization package and addons#55
adamshire123 merged 12 commits into
mit-mainfrom
NDE-95-create-github-actions-to-run-build-script-for-nde-customization-package-and-addons

Conversation

@adamshire123
Copy link
Copy Markdown

@adamshire123 adamshire123 commented May 13, 2026

Why these changes are being introduced

The workflows for testing and deploying an NDE customization package were complicated and required staff to have both the NDE development environment running locally in order to build the package and the necessary permissions and training to deploy customization packages to Primo via Alma admin.

Adding the ability to build the customization package via GitHub actions streamlines the testing and deployment workflows for our NDE customization package.

In the past, as part of code review, to fully test a new feature, a reviewer would need to pull down a local copy of the feature branch, install all of the dependencies, build the customization package, and finally upload it (or have someone else upload it if lacking permissions) to a dev view in Primo to see the results.

Likewise, when deploying changes to our production Primo instance the package would need to be built locally and then uploaded by someone with the correct Alma permissions.

(The NDE development environment offers some ability to test changes locally against our live systems via proxy, there are some quirks that make this less effective than uploading the built package to a Primo dev view for testing)

How this addresses that need

A new workflow file (.github/workflows/build.yml) is introduced that:

  • Triggers manually via workflow_dispatch, accepting a view_id input (used to name the output zip)
  • Sanitizes the view_id input before use to prevent injection issues
  • Runs the existing test workflow first (test.yml), then runs npm run build
  • Uploads both the zipped and unzipped build artifacts (dist/01MIT_INST-<view_id>.zip and the unzipped folder) using actions/upload-artifact

Now Enterprise Systems staff can simply download the built customization package from GitHub once it is ready to upload to test or deploy to production.

Additionally, .github/workflows/test.yml is updated to support workflow_call, allowing it to be reused as a dependency of the new build workflow.

Side effects of this change

The test.yml workflow now also runs when called by another workflow (workflow_call), in addition to on every push. No dependency changes — the workflow relies on the existing npm ci and npm run build commands already defined in the project.

Relevant ticket(s)

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a manual GitHub Actions build workflow for the NDE customization package and makes the existing test workflow reusable so it can gate the build.

Changes:

  • Added workflow_call support to the test workflow.
  • Added a manual build workflow that runs tests, installs dependencies, builds with a provided view_id, and uploads the generated zip artifact.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
.github/workflows/test.yml Allows the test workflow to be invoked by other workflows.
.github/workflows/build.yml Adds a manual workflow for testing, building, and uploading the NDE customization package.
Comments suppressed due to low confidence (1)

.github/workflows/build.yml:45

  • The upload path duplicates the institution ID instead of deriving it from the same source as the build script. postbuild.js names the zip using INST_ID from build-settings.env, so any future change to that setting would create a different file and this upload step would miss it.
          path: dist/01MIT_INST-${{ inputs.view_id }}.zip

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/build.yml Outdated
Comment thread .github/workflows/build.yml
Comment thread .github/workflows/build.yml Outdated
adamshire123 and others added 5 commits May 13, 2026 15:46
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…b-actions-to-run-build-script-for-nde-customization-package-and-addons' into NDE-95-create-github-actions-to-run-build-script-for-nde-customization-package-and-addons
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

.github/workflows/build.yml:58

  • This unzipped artifact path also hard-codes the institution ID while the build output directory is named from process.env.INST_ID in postbuild.js (postbuild.js:7-8). Keeping the upload path independent of the build setting makes the workflow fail if the configured institution ID changes.
          path: dist/01MIT_INST-${{ steps.sanitize.outputs.view_id }}/

Comment thread .github/workflows/build.yml Outdated
Comment thread .github/workflows/build.yml
Comment thread .github/workflows/build.yml Outdated
Comment thread .github/workflows/build.yml Outdated
adamshire123 and others added 2 commits May 13, 2026 16:11
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@adamshire123 adamshire123 requested a review from cabutlermit May 14, 2026 15:58
@adamshire123
Copy link
Copy Markdown
Author

@cabutlermit tagging you for code review, and noting that I haven't actually tested the build workflow because apparently it needs to be on the default branch before it can be run manually.

@cabutlermit
Copy link
Copy Markdown

@cabutlermit tagging you for code review, and noting that I haven't actually tested the build workflow because apparently it needs to be on the default branch before it can be run manually.

@adamshire123 - I noticed that you still have this as a Draft PR -- are you really ready for me to review it while it is still in "draft" mode? or are there more changes coming that I should wait for?

@adamshire123 adamshire123 marked this pull request as ready for review May 14, 2026 19:59
@adamshire123
Copy link
Copy Markdown
Author

@cabutlermit tagging you for code review, and noting that I haven't actually tested the build workflow because apparently it needs to be on the default branch before it can be run manually.

@adamshire123 - I noticed that you still have this as a Draft PR -- are you really ready for me to review it while it is still in "draft" mode? or are there more changes coming that I should wait for?

ready now.

Copy link
Copy Markdown

@cabutlermit cabutlermit left a comment

Choose a reason for hiding this comment

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

Let's start here with this set of requested changes (I really don't think you want to link the two workflows together).

Also, I totally feel your pain with testing new workflows -- and you are right, the .yml file must exist in the default branch before you can muck around with live testing via workflow_dispatch. When I've been in this situation in the past, I create a base workflow that doesn't really do anything other the actions/checkout and get that approved and merged into the default branch. Then, I go back and create a new feature branch where I can muck around with changes to the workflow and force manual runs in my feature branch.

Comment thread .github/workflows/test.yml Outdated
Comment thread .github/workflows/build.yml Outdated
Comment thread .github/workflows/build.yml Outdated
Comment on lines +5 to +9
inputs:
view_id:
description: 'View ID (used to name the output zip)'
required: true
default: 'NDE'
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It's not clear to me why you might need this as an input variable to the workflow? Will you ever be using a name other than "NDE" in the .zip file that gets generated by the workflow?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

We need to match the top level folder name in the zip file to the view ID of the Primo view to which the code is being deployed. so if I wanted to deploy the code to Primo view NDE_DEV I would need to have the folder in the zip file named NDE_DEV. the zip file can technically be named anything, but it makes sense to name it the same as the folder so you know what is in it without having to open the zip file to check.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ok, that makes sense why you would need this as an input for the workflow_dispatch run.

Our typical GitHub-flow works as follows:

  • opening a PR from a feature branch into the default branch deploys the app into "development" environment
  • merging the PR from a feature branch into the default branch deploys the app into the "staging" environment
  • tagging a release on the default branch deploys the app into "production"

In that model, we typically have three workflow files -- one for "development" deploys (using the on.pull_request trigger), one for "staging" deploys (using the on.push trigger, restricted to pushes to the default branch), and one for "production" deploys (using the on.publish trigger). In that model, it seems that the workflow could have the view_id hardcoded (at least for stage & prod) since you know exactly where those files/folders are going, right?

And, one more question -- how does the npm run build know how/what to name the top level folder that eventually ends up inside the .zip file?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

npm run build uses the VIEW_ID environment variable to name that folder.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I can't think of a good reason NOT to hard code the view_id when on.publish is triggered from the production branch. We don't really have a staging environment for Primo, so I'm not sure if it makes sense to differentiate between a push to the production branch and publishing from the production branch. I can see a world where we don't care about on.publish and just trigger a hardcoded view_id build for pushes to our mit-main branch. Happy to hear differing opinions on that, of course.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think we definitely want to be able to build from the feature branch without a PR from the feature branch. There are occasions where we need to build and deploy the code to a test Primo view before we're ready for a PR.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

npm run build uses the VIEW_ID environment variable to name that folder.

Sorry I'm so slow on the uptake here!

In the build-settings.env, you set the two required values (INST_ID and VIEW_ID), so if you did not do anything in this workflow file (no input, no sanitizing, no setting an environment variable for the npm run build step), wouldn't it just default to VIEW_ID=NDE from the build-settings.env file? I'm basing this on the Build the project section of the main README file.

It's not clear to me what happens if the VIEW_ID is set as an environment variable in the shell and that value is different from the value set in the build-settings.env document -- does the environment variable set in the shell override the value in the build-settings.env file? That very well might be the case, but I don't know if that's true or where that is documented.

It seems to me that that only times that you will be using the workflow_dispatch trigger would be for testing a build, not for a production deployment. Does that seem reasonable? If so, it seems that for this workflow, you simply want VIEW_ID=NDE_DEV, right?

And finally, I realized that you have both required: true and default: NDE set. I'm pretty sure that the default value will never get used since you made this a required input. If you really want it to default to NDE, you'd want required: false. Or, set required: true and don't set a default value and force the person triggering the workflow to always have to enter something.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I can't think of a good reason NOT to hard code the view_id when on.publish is triggered from the production branch. We don't really have a staging environment for Primo, so I'm not sure if it makes sense to differentiate between a push to the production branch and publishing from the production branch. I can see a world where we don't care about on.publish and just trigger a hardcoded view_id build for pushes to our mit-main branch. Happy to hear differing opinions on that, of course.

I think we definitely want to be able to build from the feature branch without a PR from the feature branch. There are occasions where we need to build and deploy the code to a test Primo view before we're ready for a PR.

You do have two different Primo environments, right? One for dev/test and one for production?

If that's true, then it seems that following makes sense:

  1. A workflow_dispatch trigger that can be run to generate a .zip build for testing (before getting to a pull request).
  2. That same workflow can also have a build-on-pull-request-to-mit-main trigger so that you automatically get a build for Primo-test/dev when your PR is opened.
  3. A merge-to-mit-main trigger to automatically build the package that should be uploaded to the Primo-test/dev environment.
  4. A release-on-main trigger to automatically build the package that should be uploaded to the PRrmo-production environment.

The first three would all have VIEW_ID=NDE_DEV since that will always be where those build artifacts should get deployed. And the fourth would automatically have VIEW_ID=NDE since it should only get deployed to production.

Copy link
Copy Markdown
Author

@adamshire123 adamshire123 May 18, 2026

Choose a reason for hiding this comment

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

The workflow is overriding the view_id in build-settings.env with an environment variable in the shell. This is how dotenv works by default. https://github.com/motdotla/dotenv#override

I will add a comment explaining this behavior to build.yml

Yes, workflow_dispatch would only be used for testing a build, not for a production build.

In Primo we're able to create as many view codes as we need. Sometimes we need to do this to test some primo setting or some customization package in isolation. For example, last week the UX team was reviewing labels which are displaying in our NDE_DEV view (these labels are configured in Primo, not in the customization package). Simultaneously, I was working on trying to add matomo tracking code. I knew the matomo code was likely to break Primo, so rather than break the NDE_DEV view while other folks were working with it, I made a NDE_MATOMO view where I could safely test the matomo code.

All that is to say, I think it is useful to have the ability to run the build with an arbitrary view_id from a feature branch.

I believe that when you specify the workflow input as required AND specify a default, the input is just pre-filled in the UI with the default.

I'll change the default to NDE_DEV, which is the most common choice.

@adamshire123 adamshire123 requested a review from cabutlermit May 15, 2026 18:32
Comment thread .github/workflows/build.yml Outdated
@adamshire123 adamshire123 requested a review from cabutlermit May 15, 2026 19:30
@cabutlermit
Copy link
Copy Markdown

@adamshire123

I think this is in good-enough shape at this point to merge it into the default branch so that we have a workflow there to do some testing of the workflow_dispatch trigger and verify that the artifacts get published properly to GitHub and are accessible.

I will admit that I'm still slightly confused on the "deployment" process -- you PR description talks about uploading files to Alma/Primo, but I thought that the artifacts from this build where getting published to the CDN (and that they only thing published to Alma/Primo was the URL to the CDN).

Copy link
Copy Markdown

@cabutlermit cabutlermit left a comment

Choose a reason for hiding this comment

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

This is good enough for now. Once this is merged, we can run some tests of the workflow_dispatch trigger and then start planning for the next phase of automatic triggers.

@adamshire123
Copy link
Copy Markdown
Author

@adamshire123

I think this is in good-enough shape at this point to merge it into the default branch so that we have a workflow there to do some testing of the workflow_dispatch trigger and verify that the artifacts get published properly to GitHub and are accessible.

I will admit that I'm still slightly confused on the "deployment" process -- you PR description talks about uploading files to Alma/Primo, but I thought that the artifacts from this build where getting published to the CDN (and that they only thing published to Alma/Primo was the URL to the CDN).

There are two different ways we can add custom code to Primo.

Customization Package

  • We intend to use a customization package to contain all of our modifications to the layout and styles of Primo UI.
  • You can only have a single customization package per VIEW_ID
  • To deploy this we upload the built .zip file to a particular Primo view via the Alma admin interface

Add-on

  • We intend to create add-ons for more complex features (TACOs for example)
  • You can have multiple add-ons per view.
  • To deploy an add-on we build the code and then host the unzipped code in the CDN.

@adamshire123 adamshire123 merged commit 6a2d7e6 into mit-main May 18, 2026
1 check passed
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.

3 participants