Repair config issues

[samuelcambien]

References

• Fixes yarn start:dev - Environment variables, YML configurations no longer appear to be honored for "ui" settings #1778
• Fixes Default settings in config/config.yml are ignored #1795

Description

Issue #1795: The config building mechanism has been updated to always include the default config from config.yml first. This config is then overwritten by environment config (e.g. config.dev.yml).
Issue #1795: : The 'yarn run serve' command has been repaired to use the proper 'ui' config.

Instructions for Reviewers

List of changes in this PR:

• The 'buildAppConfig' method in config.server.ts has been updated to always use the default config.yml first
• Getting the default config.dev.yml path has been moved to a separate method 'getDefaultConfigPath', the 'getLocalConfigPath' method has been renamed to 'getEnvConfigFilePath'.
• The 'yarn run serve' command has been updated to use the serve.ts file
• The cli parameters in this file have been updated with '--configuration development'
• The version in package.json has been updated: 7.4.0-next.0 instead of 0.0.0 (this follows the Angular versioning system)
• This version has been added to the message which prints the environment (Production/Development)
• Printing this message has been moved, so it is no longer printed at every request for the server app

Expected results:

• If there exist both a config.yml and a config.dev.yml file, when running the project in the development environment, the config from both files should be used. The config from config.dev.yml has priority though. Same principle for test and prod environments.
• When running 'yarn run serve' or 'yarn start:dev', the proper ui config should be used according to the principle above.
• When running 'yarn run serve' or 'yarn start:dev', the source files should be available in the dev tools debugger.
• When running the project in test or prod mode, the file 'serve.ts' should not be used.
• The version (7.4.0-next.0) should be printed in the console, above the environment message

Checklist

• My PR is small in size (e.g. less than 1,000 lines of code, not including comments & specs/tests), or I have provided reasons as to why that's not possible.
• My PR passes TSLint validation using yarn run lint
• My PR doesn't introduce circular dependencies
• My PR includes TypeDoc comments for all new (or modified) public methods and classes. It also includes TypeDoc for large or complex private methods.
• My PR passes all specs/tests and includes new/updated specs or tests based on the Code Testing Guide.
• If my PR includes new, third-party dependencies (in package.json), I've made sure their licenses align with the DSpace BSD License based on the Licensing of Contributions documentation.

[artlowel]

A bit of a clarification on the version number. I added that after reviewing this code, as it has bugged me many times before that there's no real way of knowing which version of the UI you're dealing with unless you know the exact history of when which feature was merged.

The source of the version number is package.json. Which means we'll have to keep this up to date for every release. I used the same system as angular does. -next means the version that's currently being worked on. We could replace that with rc for a release candidate and leave out the suffix for a stable release

I also moved the "Enviroment: Production" message here, as on the current master it is put in the server log for every page request. With this PR it will be logged only once. When the server starts the first time.

[tdonohue]

@samuelcambien : This has minor merge conflicts now. Could you rebase it when you get the chance? Thanks!

[samuelcambien]

Hi Tim, I've rebased the branch and resolved the merge conflicts. Regards, Samuel

[tdonohue]

@samuelcambien and @artlowel : First off, this works well. The bugs are no longer reproducible.

My only request though is to change the version tag added by @artlowel . See my reasoning inline below. Assuming you agree, then this is ready to merge once that tag is updated. If you disagree, we can always discuss this in more detail in our next meeting.

[tdonohue]

@samuelcambien and @artlowel : Based on my reviewing of the versioning docs at https://semver.org/ and https://classic.yarnpkg.com/en/docs/dependency-versions#toc-semantic-versioning, I'd prefer if we changed this to:
7.4.0-next

My reasoning is that the .0 after the -next appears to imply that we'd have multiple -next pre-releases. So, I'd prefer we only use that if we plan to ever do beta releases... e.g. 7.4.0-beta.1 and 7.4.0-beta.2 seem reasonable. But, -next to me should be more like a -SNAPSHOT on the backend...and it's never going to be incremented like -next.0, -next.1, etc.

I also gave this versioning scheme a test using yarn version... and it says that 7.4.0-next is valid SemVer.

Once we agree on a strategy, I'll change our Release Procedure to remind us (mostly myself) to run yarn version prior to any new release in order to update this version tag.

[artlowel]

Sounds good. I've changed it

[tdonohue]

👍 Thanks @samuelcambien and @artlowel ! This looks great to me now.

I've also updated our release procedure instructions to include instructions for using yarn version to update the "version" in package.json: https://wiki.lyrasis.org/display/DSPACE/Release+Procedure#ReleaseProcedure-ReleasetheFrontend(UI)viaaGitHubReleaseTag

[atarix83]

thanks @samuelcambien for this PR

I verified everything is working fine, mainly on cofiguration overwriting

[tdonohue]

Merging as this is at +2. Thanks again @samuelcambien and @artlowel !