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

Implement Sierra's "just cataloged" feature #320

Closed

Conversation

julia-bauder
Copy link
Contributor

The Sierra OPAC has several statuses beyond just "available" or "checked out." One of these statuses is "just cataloged," which is applied to items for a library-defined number of hours after the item has been cataloged. It alerts users and circulation desk staff that the item is likely not yet on the shelf, but instead should be searched for at locations along the workflow between cataloging an item and placing it on the shelf. These changes allow VuFind to mirror the "just cataloged" status in the OPAC.

@demiankatz
Copy link
Member

Thanks for sharing this! A couple of comments:

1.) Do you think it's worth creating a protected getLocationText() method to reduce redundancy?

2.) Would you mind adding some isset() checks and reasonable defaults in case somebody has a legacy configuration where the new settings are not yet defined?

If you think these things are good ideas but don't have time to implement them, I'm willing to do this when I get a bit more time of my own. Either way, I'll hold off on merging this until after Monday's release!

@julia-bauder
Copy link
Contributor Author

I think those are both good suggestions. I should have time to make those
changes tomorrow.

Thanks!

On Thu, Mar 19, 2015 at 9:35 AM, Demian Katz notifications@github.com
wrote:

Thanks for sharing this! A couple of comments:

1.) Do you think it's worth creating a protected getLocationText() method
to reduce redundancy?

2.) Would you mind adding some isset() checks and reasonable defaults in
case somebody has a legacy configuration where the new settings are not yet
defined?

If you think these things are good ideas but don't have time to implement
them, I'm willing to do this when I get a bit more time of my own. Either
way, I'll hold off on merging this until after Monday's release!


Reply to this email directly or view it on GitHub
#320 (comment).

@demiankatz
Copy link
Member

Great, thanks!

@julia-bauder
Copy link
Contributor Author

Updated as suggested.

@demiankatz
Copy link
Member

Thanks! I'll get this merged next week!

@demiankatz
Copy link
Member

Okay, I've manually merged this to master.

I made a few changes to the getLocationText() to get rid of code style warnings (the way the long lines were structured, I had to refactor a bit to make things fit). Hopefully I haven't broken anything, but if you don't mind giving it a once-over, I'd appreciate the double-check!

Thanks again for sharing!

@demiankatz demiankatz closed this Mar 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants