dhis2-devs team mailing list archive

[Dave Trombley <dave.trombley@xxxxxxxxx>]

   Hi all -


     We came across an odd situation during testing our implementation of
DHIS.  I think there might be a flaw in the actual model of DataElement -
specifically: the interfaces are happy to allow you to create multiple
datasets, each with different period types, and assign the same dataelement
to them.   However, the core apis seem to want to impose a *-1 relationship
from data elements to period types!  For example:

    org/hisp/dhis/dataelement/DefaultDataElementService.java: 281

    /**
     * Returns the PeriodType of the DataElement, based on the PeriodType
of the
     * DataSet which the DataElement is registered for.
     */
    public PeriodType getPeriodType()
    {
        return dataSets != null && dataSets.size() > 0 ?
dataSets.iterator().next().getPeriodType() : null;
    }

   So code using this model will only "see" one of the data element's
various period types.  (possibly worse - is there any guarantee it will
always be the same one?)

     We encountered this via a real use case failing catastrophically -
here's how to reproduce it

  - Create a data element A
  - Create a data set B, give it Monthly type and assign A to it.
  - Notice that in the Data Validation Rule creation screen, while
attempting to create a new validation rule with "Monthly" period, A shows
up in the list of available elements.  It does not show up while creating a
'Yearly' data validation rule.
   - Create a data set C, give it Yearly type and assign A to it.
   - Now A will show up as available in exactly one of those two data
validation rule screens, but not both.  The presence of a completely
unrelated dataset will prevent an admin from completing the validation
rules for the dataset they've been working on.

   It actually took us a bunch of hours to track this down!   I'd be happy
to submit a patch - to my thinking there should be something like

   public boolean isInDataSetWithperiodType(PeriodType pt);

   the replaces the above.   However, I don't know the rest of the model
very well, so it might be more appropriate to forbid the same DE from being
assigned to multiple DS with varying period types...  or there might be
some other fix...

   Thoughts?

   -David