-
-
Notifications
You must be signed in to change notification settings - Fork 825
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
[NFC] dev/core#1466 Update Documentation URLS to be the correct links… #16085
Conversation
… in the security component check
(Standard links)
|
@@ -364,7 +364,7 @@ public function isDirAccessible($dir, $url) { | |||
* @return string | |||
*/ | |||
public function createDocUrl($topic) { | |||
return CRM_Utils_System::getWikiBaseURL() . $topic; | |||
return CRM_Utils_System::docURL2('sysadmin/setup/security#' . $topic, TRUE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return CRM_Utils_System::docURL2('sysadmin/setup/security#' . $topic, TRUE); | |
return CRM_Utils_System::docURL2('sysadmin/setup/security#' . $topic, TRUE); |
where is "docurl2" defined as a function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -71,7 +71,7 @@ public function checkLogFileIsNotAccessible() { | |||
$url[] = $log_path[1]; | |||
$log_url = implode($filePathMarker, $url); | |||
if ($this->fileExists($log_url)) { | |||
$docs_url = $this->createDocUrl('checkLogFileIsNotAccessible'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I enter https://wiki.civicrm.org/confluence/display/CRMDOC//uploads-should-not-be-accessible I get a 404 - unchanged I get redirected to the security page on the docs - ie the equivalent of
return CRM_Utils_System::docURL2('sysadmin/setup/security#' . $topic, TRUE);
So this change make it un-better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isn't that what L367 is meant to be doing?)
Betty and I are reviewing PR's and we came across this PR. We are unable to reproduce this PR as it is not clearly explained. However we do think this PR is ready to be merged as it is only updating non-working links to the CiviCRM documentation. So not a risk of breaking anything. And if it breaks then it breaks existing broken links. @eileenmcnaughton or @mattwire could one of you merge this PR? |
Merging based on @jaapjansma review @seamuslee001 please follow up with a subsequent PR if there are still broken links (per @eileenmcnaughton comment above) |
Note: We can refactor the underlying system here next year - we'll be adding metadata (catogorisation/tags) to docs (2020) and an API to pull docs information in UI along with an extension using it planned for 2021. |
… in the security component check
Overview
Updates the documentation links from pointing at the Wiki to be pointing at the mkdocs
ping @MikeyMJCO @eileenmcnaughton