Islandora / controlled_access_terms

Drupal module for subject and agents
GNU General Public License v2.0
7 stars 30 forks source link

Fix validate check #77

Closed nigelgbanks closed 2 years ago

nigelgbanks commented 2 years ago

Fixes validation in two small cases Empty strings are now always prohibited and checking for intervals does not error if a non-interval value is given.

Previously:

use Drupal\controlled_access_terms\EDTFUtils;
EDTFUtils::validate("", TRUE, TRUE, TRUE);
// yields [];
EDTFUtils::validate("", FALSE, TRUE, TRUE);
// yields [ "Could not parse the date ''." ]
EDTFUtils::validate("1985-04-12T23:20:30Z", TRUE, TRUE, TRUE);
// yields [ "Date intervals cannot include times." ]

Now:

use Drupal\controlled_access_terms\EDTFUtils;
EDTFUtils::validate("", TRUE, TRUE, TRUE);
// yields [ "Cannot parse empty value." ];
EDTFUtils::validate("", FALSE, TRUE, TRUE);
// yields [ "Cannot parse empty value." ];
EDTFUtils::validate("1985-04-12T23:20:30Z", TRUE, TRUE, TRUE);
// yields [ ]
seth-shaw-unlv commented 2 years ago

Happy to review, although it probably won't be until tomorrow. In the mean-time we need to address those failing builds. I know we recently saw this on other repos, but I don't recall what changes we made to the github actions config to fix it.

whikloj commented 2 years ago

You need to uninstall mysql-client and mysql-common before trying to install it. https://github.com/Islandora/islandora/pull/861/files#diff-453d9c758046a0eacc2d4f41b1f72b223fe486676bc31b09dbddc94188ce434dR177

seth-shaw-unlv commented 2 years ago

@nigelgbanks, my last day at work before paternity leave is this Friday. Could you make the build changes before then so I can merge this before I check-out for a few weeks?

nigelgbanks commented 2 years ago

Tried to apply the changes that @whikloj suggested and it got past the MySQL installation, but now just hangs on the Setup Drupal step. I can't restart the job or stop it since I'm not an admin of the repository. From what I've read it may take 6 hours to stop on it's own (https://github.community/t/step-hangs-indefinitely/17720).

nigelgbanks commented 2 years ago

@seth-shaw-unlv & @whikloj for this to get fixed this other pull must be first merged:

https://github.com/Islandora/islandora_ci/pull/8

I've tested it on a my own fork (which is passing): https://github.com/nigelgbanks/controlled_access_terms/runs/4934638675?check_suite_focus=true

After that has been merged you'll have to manually rerun the Github Actions in the pull request.