In D6, we have a custom table called {project_release_supported_versions} that records info about branches of development for each project. This is were we store stuff like what branch(es) are supported, recommended, the latest release for each branch (computed automatically), should -dev snapshots be shown on the project page, etc.

We need to decide how we want to handle this in D7. A few possible options I've come up with while working on this:

A) Use a field_collection attached to project nodes. This gives us a fieldable entity that we can add our fields to. Some of them have a hidden widget (e.g. all the entity references to latest/recommended release nodes for each branch) and we populate those programmatically. Others are just plain old checkbox bools or whatever that the project maintainer can currently toggle at node/%/edit/releases. However, we really want the "recommended branch" thing to be a set of radios across all minor versions of the same major version (if on d.o we think of the "7.x" part as the major and "1" as minor), and that's not easy to do with just clicking this together via field_collection. Also, field_collection makes the UI when editing the project node kinda crappy, since if you have a multi-valued field collection, and it contains any required fields, you can't just edit the project without removing the default new field_collection it gives you. This is probably a bug in the field_collection queue, I'd need to search.

B) Basically keep what we have now. In some ways, this is a bit crappy, but our needs for this are fairly Special(tm) and trying to bend field_collection to our will might end up being harder than just keeping our custom code for this. Sadly, if we go this route, we have to keep/port/maintain a bunch more views code than I was hoping for, in addition to the DB table and all the code to deal with it.

Either way, we desperately want to kill the whole insanity with "API compatibility tid" + version_major as the key into this data. We just want a nice happy "branch identifier" string or something. I'm honestly still a bit on the fence about the right way to represent this. :/ We certainly need to be able to figure out what branch a given release is from, so that when a new release is out, we know what branch to update the "latest release" info for, etc. But, I hate the idea of using tid + major like we do in D6. Down with taxonomy magic in project_release!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dww’s picture

Mostly as a backup, here's the field_inspector output from the field_collection I clicked together when starting down path A. This would need to be folded into project_release.install and includes/project_release_node_type.inc to actually be a useful patch, but since I already spent the time trying to define this as a proof-of-concept, I wanted to share the results here for backup and possible collaboration if anyone else wants to look into this, too.

drumm’s picture

Assigned: dww » drumm

A version of the field_release_branches landed in the 1551384-release-deep-port branch, named field_project_has_releases with #1546092: Add a field to control if a given project node has releases.

The rest of the fields look generally good, but I'll pull them in as I understand them.

I like the idea of getting away from taxonomy magic. It looks like the list of available API compatibility versions, like 6.x, 7.x, 8.x, is used frequently in the code. I think we want a project node type setting for this. Maybe a text area next to the "Download link base URL" field, which sets the options one-per-line. Then, release nodes either have a select field for API compatibility, with #options from the project node type; or it is used in validation for field_branch_identifier.

drumm’s picture

I went ahead and added a per-release-node-type setting for API compatibility options, http://drupalcode.org/project/project.git/commit/9a24745, as described in #2. I'll work on using it throughout the code.

drumm’s picture

dww and I talked about this. What we tentatively settled on is

  • Do a straight upgrade, keeping project_release_supported_versions as a custom table. Do a straight upgrade of the same UI, views integration, etc.
  • Project release nodes will use more simple fields, notably a text field for the whole version number, rather than a bunch of little fields assembled into a version number. This is programmatically split out into components (api, major, patch, extra) for use in project_release_supported_versions and anywhere else needed. The version string is already in the branch.

This will allow us to move forward with the upgrade with an architecture that we can live with. Rearchitecting it to be more-cleanly field-based is something we can think about doing after the 7.x upgrade.

dww’s picture

Also, we agreed to just keep using taxonomy for the "api compatibility", and we'll revert commit 9a24745 from #3. It's kinda crappy to use taxonomy, but there are a *lot* of places on both releases and projects that use that, and it's just going to be less total work to basically port the system we have now.

Ideally, while we're porting, we could be untangling the dependency on taxonomy so that sites which don't care can just use simple major.minor.patch versions and not have to think about or use the taxonomy stuff. But, if that slows us down too much, we might just have to live with the dependency for now and circle back later (hopefully!)...

dww’s picture

p.s. We also discussed more or less keeping the "version string format" setting, but using it in the opposite direction -- to parse the single version string field from the release node to figure out what branch it belongs to for the purposes of {project_release_supported_versions}. Or something. Exposing a regexp to end-users seems evil and wrong, and perhaps the existing token-based system (with the funky punctuation prefix stuff) is the best UI for it.

drumm’s picture

I've gotten into rewriting this

function project_release_query_releases_by_branch($project_nid, $api_tid, $major, $access = FALSE) {
  $api_vocabulary = taxonomy_vocabulary_load(variable_get('project_release_api_vocabulary', ''));
  $query = new EntityFieldQuery();
  $query->entityCondition('entity_type', 'node')
    ->entityCondition('bundle', 'project_release')
    ->propertyCondition('status', 1)
    ->fieldCondition('field_release_project', 'target_id', 1688574)
    ->fieldCondition('taxonomy_' . $api_vocabulary->machine_name, 'tid', 7234)
    ->fieldCondition('field_release_version_major', 'value', 3)
    // We always want the dev snapshots to show up last.
    ->fieldOrderBy('field_release_build_type', 'value', 'DESC')
    // Sort by the obvious integer values along the branch (minor and patch).
    //todo
    //$order_bys[] = 'r.version_minor DESC';
    //$order_bys[] = 'r.version_patch DESC';
    // To reliably sort release with version_extra, use version_extra_weight.
    //$order_bys[] = 'r.version_extra_weight DESC';
    // Within releases of the same version_extra_weight (e.g. rc1 vs. rc2),
    // sort by version_extra_delta.
    //$order_bys[] = 'r.version_extra_delta DESC';
    // Within releases of the same version_extra_weight and version_extra_delta,
    // sort alphabetically. This shouldn't normally happen, but just in case you
    // have multiple releases with the same delta (e.g. "alpha-one", "alpha-two"
    // etc), at least you'll get deterministic results.
    //$order_bys[] = 'r.version_extra DESC';

  // Only enforce node access via db_rewrite_sql() if the caller specifically
  // requested that behavior.
  if (!$access) {
    $query->addTag('DANGEROUS_ACCESS_CHECK_OPT_OUT');
  }

  $result = $query->execute();
  return $result['node'];
}

I've already made field_release_version_major a field for that condition, and maybe other uses. I've been thinking about a few ways to tackle the commented out order bys:

  • Make fields for minor, patch, and extra, in a way that allows correct sorting on extra. Good if we need these for something else.
  • Make a sort text field which concatenates all that just for this sort. Like 000012-000001-002-0001-rc, minor-patch-extra_weight-extra_delta-extra, with ints padded out.
  • Load everything and sort in PHP. This would be slow.
drumm’s picture

Status: Active » Needs review

I settled on making fields for minor, patch, and extra. I committed these to the branch along with updating various code in the node save path.

Since release nodes are saving well, I think I may be able to change the status.

drumm’s picture

Status: Needs review » Fixed

Marking this as fixed since 1551384-release-deep-port has been merged.

Automatically closed -- issue fixed for 2 weeks with no activity.