NCEAS / metacatui

MetacatUI: A client-side web interface for DataONE data repositories
https://nceas.github.io/metacatui
Apache License 2.0
42 stars 28 forks source link

Remove EML version specific language from filenames and models #1424

Open amoeba opened 4 years ago

amoeba commented 4 years ago

(I'm writing this up so others on the team can see how we could do this but we may decide not to once we see what it involves).

We want to support EML 2.2.0 in the editor but not support n past versions of EML in the same codebase. EML is designed such that old versions can be migrated to the newer version via transformation. So we can make all of the Backbone models for the editor support 2.2.0 docs and migrate any older docs forward so the codebase only supports the latest released version of EML. Good so far.

Our Backbone models are in a folder src/js/models/metadata/eml211/{EML211.js,EMLProject.js,EMLParty.js,etc}. And the main EML model is in a file called EML211.js. Since we're supporting EML 2.2.0 now it'd be nice to take the "211" out of all of these names.

We could just rename everything and be done with it but we may have downstream users depending on files being at certain paths so moving things from an eml211 subfolder to, say, a version-agnostic eml folder and renaming EML211.js EML.js could break things. I'm specifically thinking of ESS-DIVE.

To make this happen, we propose to keep all of the old EML model JS files in their same place and replace them with a simple shell model which extends the model which exists at the new place. e.g., EML211.js becomes:

// EML211.js
define(["models/metadata/eml/EML"], function (EML) {
  var EML211 = EML.extend({});

  return;
});

This works for both views (e.g., EML211EditorView.js) and models (e.g., EML211.js). Any downstream users can require (in the RequireJS sense) a deprecated path like src/js/models/metadata/eml211/EML211.js and they'll get a model which extends src/js/models/metadata/eml/EML and acts just like that class (which is a copy of what's now at EML211.js).

@laurenwalker asked to see the class in the docs/design folder updated with how all of these models would relate but PlantUML wasn't up for the task (or at least I couldn't get such a complex layout to render nicely). So I simplified it:

Screen Shot 2020-06-04 at 8 02 54 PM

This same thing works for views. Our application requires stuff from the top row any users with existing hard references to stuff in the bottom row still work because the behavior of the models and views are identical.

I've already done this for the codebase and tested things out. It works great. I did it in two steps on a feature branch so it's relatively clean to see what I did (I hope):

Commit 1: https://github.com/NCEAS/metacatui/commit/0dd0116e7529189315728eed9e01c5829094934d: Copy everything, updating paths and names appropriately, and turn all old models into their simple shell version which just extend the new, version-agnostic Model or View. Add @deprecated flags to all of these and throw a README in the eml211 folder indicating as much.

Commit 2: https://github.com/NCEAS/metacatui/commit/343bb51b2c0a70566481c05367b045ad12d2d876: Update the application to reference all of the new model locations and remove references to things like EML211 or EML211.js in the code including comments.

I like how this is working so far and can apply my existing changes upgrading everything to support EML 2.2.0 on top of this branch. In MetacatUI version 3 (or earlier) we could plan to remove all of these renamed models and views for a cleaner codebase but for now we don't break anything.

@csjx @laurenwalker how does that sound? I'm happy to fill in more detail anywhere this overview didn't cover.

mbjones commented 4 years ago

I like the plan in principle.

laurenwalker commented 4 years ago

This looks good to me. Thanks for the additional info. Let's just make sure we do a lot of testing on the display and editing of EML 2.1.1 and EML 2.2.20 docs before merging into a dev branch.

amoeba commented 4 years ago

Now that you bring up the merge, I think we'll have a lot of trouble if we do that and spend more than a day testing on a feature branch before merging because of the path changes involved. I think the best way to go would be to save my work here, get the above two commits on dev-2.12 quickly, and add any work on top of that. I could do that work over next Monday so it doesn't hold up any other v2.12 work. Does that sound okay?

amoeba commented 4 years ago

@laurenwalker and I chatted in Slack just now and we're going to wait until the 2.12 branch is tagged, the 2.13 branch is created, and then we'll apply this to the 2.13 branch so all 2.13 branch work is done on top of it.

amoeba commented 4 years ago

This work didn't get done but I see the issue has been closed. Are we still interested in this?

mbjones commented 4 years ago

Yes from my perspective πŸš€

laurenwalker commented 4 years ago

Yes, that was my mistake for closing it. I think it's important to do this, particularly because the EML211 model actually creates EML 2.2.0 documents πŸ˜†