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.

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

Handle InProgressEtd::NO_EMBARGO values in the actor stack #1491

Merged
merged 2 commits into from
Aug 7, 2018

Conversation

no-reply
Copy link

@no-reply no-reply commented Aug 7, 2018

The PregradEmbargo actor is responsible for setting up attributes for a
six-year pregraduation embargo when an embargo_length is passed during Etd
creation. This works more or less the same as its predecessor, but relies more
on base Hyrax behavior. The values passed here are eventually interpreted at
graduation time to determine the post-grad embargo release date.

However, the InProgressEtd data stores a specific string indicating that no
embargo is requested and passes that value to the stack. This is always
interpreted as a request for a pregrad embargo.

Ideally, we would handle this at the edge of InProgressEtd so other parts of
the application don't need to be aware of this blessed string. It's likely that
InProgressEtd needs this to be round-tripabble, and the handling for
transformation of this data lives across various methods. Rather than attempt an
immediate refactor, we fix the bug by handling the special data in the actor
stack. We raise a warning so the larger fix won't get ignored.

Fixes #1485.

@no-reply
Copy link
Author

no-reply commented Aug 7, 2018

#1492 is related, but better taken on when InProgressEtd and the form are more stable.

Tom Johnson added 2 commits August 6, 2018 17:40
This string was hard coded in a number of places, which was fragile. Some
handling of this is going to be required elsewhere (pending a more compelete
refactor here), so the constant seems necessary.
The `PregradEmbargo` actor is responsible for setting up attributes for a
six-year pregraduation embargo when an `embargo_length` is passed during `Etd`
creation. This works more or less the same as its predecessor, but relies more
on base Hyrax behavior. The values passed here are eventually interpreted at
graduation time to determine the post-grad embargo release date.

However, the `InProgressEtd` data stores a specific string indicating that no
embargo is requested, and passes that value to the stack. This is always
interpreted as a request for a pregrad embargo.

Ideally, we would handle this at the edge of `InProgressEtd` so other parts of
the application don't need to be aware of this blessed string. It's likely that
`InProgressEtd` needs this to be round-tripabble, and the handling for
transformation of this data lives across various methods. Rather than attempt an
immediate refactor, we fix the bug by handling the special data in the actor
stack. We raise a warning so the larger fix won't get ignored.

Fixes #1485.
@no-reply no-reply force-pushed the new-form-no-embargo branch from 1eea962 to a558747 Compare August 7, 2018 00:40
@no-reply no-reply requested a review from jenlindner August 7, 2018 00:40
@val99erie val99erie merged commit ea04dc7 into master Aug 7, 2018
@val99erie val99erie deleted the new-form-no-embargo branch August 7, 2018 15:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Submission with no Embargo shows as Embargo
2 participants