-
Notifications
You must be signed in to change notification settings - Fork 445
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
Refine "Public URL Identifier" availability and behaviour #5430
Comments
Note: this isn't a bug per se, but more of a potential for confusion. Time depending, if this has to wait to a later release, that's fine - but it should be reviewed at some point. @AhemNason also says that a small note attached to the field describing its intended use would also be fine. |
I'm totally behind this, but it's my understanding that this field is in use as a kind of pub id -- sometimes referencing a submission's ID in other systems or something like that. I'd love to separate these out. At the moment, the URL slug is stored in a setting called So I'm definitely up for changing this but I just want to double-check that we're not mushing data together here in a way that doesn't match its actual use. This is always a danger when changing the underlying meaning of a field. Another approach would be to keep I'm also happy to just get rid of it if we think it's not being used or shouldn't be used. |
Honestly, I think the biggest issue is that it’s called a “public id” If it
were called a “publisher id” and had some copy about what a publisher ID
might look like, I’m ok putting it there.. “public URL identifier” doesn’t
really describe what this is either.
…On Mon, Jan 27, 2020 at 6:44 AM Nate Wright ***@***.***> wrote:
I'm totally behind this, but it's my understanding that this field is in
use as a kind of pub id -- sometimes referencing a submission's ID in other
systems or something like that. I'd love to separate these out.
At the moment, the URL slug is stored in a setting called
pub-id::publisher-id and this is used in methods like getBestId(). It is
also in use by galleys, issues and issue galleys -- anywhere pub ids are in
play.
So I'm definitely up for changing this but I just want to double-check
that we're not mushing data together here in a way that doesn't match its
actual use. This is always a danger when changing the underlying meaning of
a field.
Another approach would be to keep pub-id::publisher-id in place, but make
it opt in like the DOI or other pub ids. Then to copy any existing values
over to a new field like urlPath, and use that instead. This would keep
the value in both places and let people disambiguate going forward.
I'm also happy to just get rid of it if we think it's not being used or
*shouldn't* be used.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5430?email_source=notifications&email_token=AAD3DVH7T7YHL6RZVRHKGNTQ73CLLA5CNFSM4KLIGBA2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJ7G5QQ#issuecomment-578711234>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAD3DVBPRYZFINB5SRS7TCLQ73CLLANCNFSM4KLIGBAQ>
.
|
To my knowledge, there is no such guidance. It's an extra pub id with no specific intended purpose, that we also use for the URL though we have never said as much to editors. |
Coming back to this (and cc'ing @asmecher). I think we'll need a decision on this soon to get it resolved for 3.2. A brief recap:
I can see two options:
Both options address the confusion for the end user. However, Option 1 leaves the ambiguity in the data structure and we'll probably need to resolve this some day. |
I have a suspicion that we'll be asked for both versioned and non-versioned URL slugs, and versioned and non-versioned publisher IDs. Option 2) gives us non-versioned URL slugs only, and versioned publisher IDs only (correct?). But the two are at least clear and disambiguated, so we can add more options later if need be. Of the two options, I support the second. |
I think the I'll plan to work on Option 2 unless James or Mike having anything they want to add. |
Option 2 sounds fine to me! |
I agree with James!
…On Tue, Feb 4, 2020 at 11:20 AM JB MacGregor ***@***.***> wrote:
Option 2 sounds fine to me!
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5430?email_source=notifications&email_token=AAD3DVFEMQZRG7R2BZPKNJ3RBGBUVA5CNFSM4KLIGBA2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKYAB6A#issuecomment-581959928>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAD3DVEPSUR5ANKMN64WT5DRBGBUVANCNFSM4KLIGBAQ>
.
|
This commit migrates existing usage of the pub-id::publisher-id property to a new urlPath property for all objects that have it. The pub-id::publisher-id property remains to be used separately from urlPath and has been moved to a piece of metadata. URL handlers now look in urlPath to fetch objects based on requests that don't use IDs.
This commit migrates existing pub-id::publisher id values to a new urlPath prop and uses that property for URL routing. The pub-id::publisher-id property is now a metadata field and can be enabled/disabled. Existing values were kept
This commit migrates existing usage of the pub-id::publisher-id property to a new urlPath property for all objects that have it. The pub-id::publisher-id property remains to be used separately from urlPath and has been moved to a piece of metadata. URL handlers now look in urlPath to fetch objects based on requests that don't use IDs.
This commit migrates existing pub-id::publisher-id values for publications and publication formats to a new urlPath property and uses that for URL routing. The pub-id::publisher-id property is now a metadata field and can be enabled/disabled.
This commit migrates existing pub-id::publisher id values to a new urlPath prop and uses that property for URL routing. The pub-id::publisher-id property is now a metadata field and can be enabled/disabled. Existing values were kept
PRs: Publisher ID is now enabled as metadata under Settings > Workflow > Submission > Metadata. It appears alongside other metadata. And a new field has been added for urlPath. The following tooltip gives some guidance. @AhemNason can you please give me a 👍 or 👎 on this tooltip, particularly its usage in PubMed? (That's the only place I could find the publisher ID used). Similar steps have been taken with the In OMP, this has been implemented for publication formats but not chapters or submission files. Chapters do not have URLs yet so I left the This required a bit more heavy-handed changes than I expected. I'm a bit nervous about this going in so late before 3.2's release. I'd appreciate any time that anyone has to check how the new URL Paths work. @asmecher The new |
This commit migrates existing usage of the pub-id::publisher-id property to a new urlPath property for all objects that have it. The pub-id::publisher-id property remains to be used separately from urlPath and has been moved to a piece of metadata. URL handlers now look in urlPath to fetch objects based on requests that don't use IDs.
This commit migrates existing pub-id::publisher id values to a new urlPath prop and uses that property for URL routing. The pub-id::publisher-id property is now a metadata field and can be enabled/disabled. Existing values were kept
I've merged this in but there are still two things I'd like to hear back on.
|
+1 for sure.
…On Thu, 20 Feb 2020 at 11:43, Nate Wright ***@***.***> wrote:
I've merged this in but there are still two things I'd like to hear back
on.
1. @AhemNason <https://github.com/AhemNason> can you please give me a
+1 or -1 on this tooltip for publisher ID?
<https://user-images.githubusercontent.com/2306629/74674981-70624500-51aa-11ea-9c79-260b95615ec1.png>
1. @asmecher <https://github.com/asmecher> The new url_path columns in
the db tables are all VARCHAR(32). Should we make this longer or change the
type?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5430?email_source=notifications&email_token=AAD3DVEX2IKZZZHX2S3ZSNLRD2QJFA5CNFSM4KLIGBA2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEMO2EPY#issuecomment-589144639>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAD3DVDJX2WJEXLMLIBBYGTRD2QJFANCNFSM4KLIGBAQ>
.
|
I haven't heard of a need to go beyond 32 characters, but it is an arbitrary limit and I wouldn't object to e.g. doubling it. (We used to have occasional issues with MySQL's max key length getting hit in certain tables -- that's one potential downside to increasing column lengths on indexed columns. But I don't think we're at risk of that in this case.) |
This commit migrates existing pub-id::publisher id values to a new urlPath prop and uses that property for URL routing. The pub-id::publisher-id property is now a metadata field and can be enabled/disabled. Existing values were kept
Thanks! |
This commit migrates existing pub-id::publisher id values to a new urlPath prop and uses that property for URL routing. The pub-id::publisher-id property is now a metadata field and can be enabled/disabled. Existing values were kept
Hi. I just wanted to confirm if the following field description remains valid: "A unique ID provided by the publisher. It will be used in the publication's URL path instead of the id when present." I was with the impression that for fresh installs, urlPath would be totally split from publisher IDs. Only after upgrades that any pub-id::publisher-ids would be copied over to the urlPath, so existing URLs continue to work. |
Good catch @fgnievinski! This is out of date. Now the |
Problem(s):
Proposed Solutions:
The text was updated successfully, but these errors were encountered: