Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Gh 4845 settinig lease date in past #4889

Merged
merged 3 commits into from
May 4, 2021
Merged

Gh 4845 settinig lease date in past #4889

merged 3 commits into from
May 4, 2021

Commits on May 4, 2021

  1. Fixing rendering logic for visibility component 

    Fixes #4845

Prior to this fix, if we had a new record that either set an embargo or
a lease, we would narrow our rendering of the visibility component to
only a partial related to the embargo or lease.

```gherkin
Given a form that set an embargo (or release date)
And that form had invalid data
When I submitted the form
Then the response form would inidicate there was invalid data
And would render only the portion of the visibility related to
  embargo (or release date if set)
```

With this change, the form renders the entire visibility component,
letting the user change their visibility options (e.g., go from with
Lease to Public or what have you).  However, the error messaging is not
providing insight into the problem; That is to say there's a red flash
error, but the content is empty; and there is no indicator on the lease
field that there existed an error.  This warrants further work, and will
be a commit that builds on this change.
    @jeremyf
    jeremyf committed May 4, 2021
    Configuration menu
    Copy the full SHA
    e1fd224 View commit details
    Browse the repository at this point in the history

  2. Adding rendering of lease related error messages 

    Prior to this commit, the `form_visibility_error` partial did not
include the errors for the `:lease_expiration_date` nor the
`:visibility_after_lease`.  Per convention of Hyrax, we place the error
messages on the form and not in close DOM proximity to the HTML field
in which we encountered the error.

This relates to #4845
    @jeremyf
    jeremyf committed May 4, 2021
    Configuration menu
    Copy the full SHA
    9511dce View commit details
    Browse the repository at this point in the history

  3. Adjusting lease/embargo_enforced helper logic 

    This reflects moving the change from e1fd224 — "Fixing rendering logic
for visibility component" into the helper method.  This suggestion comes
from [no_reply's review comment][1], and makes sense.  I don't think we
want to push the logic further down the stack, as there's a case
statement that would add complexity, as well as it may have unintended
consequences.  The change at this level (e.g., a helper method) makes it
more clear that this is for rendering UI components.

Related to #4845.

[1]:https://github.com/samvera/hyrax/pull/4889/files#r625466486
    @jeremyf
    jeremyf committed May 4, 2021
    Configuration menu
    Copy the full SHA
    adb14ec View commit details
    Browse the repository at this point in the history