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

[FIX] Downloading files when running in sub directory #14485

Merged
merged 12 commits into from
May 20, 2019

Conversation

miolane
Copy link
Contributor

@miolane miolane commented May 13, 2019

fix URI for uploading file

Closes #14472

fix uri of upload file
@CLAassistant
Copy link

CLAassistant commented May 13, 2019

CLA assistant check
All committers have signed the CLA.

@miolane miolane closed this May 13, 2019
@miolane miolane reopened this May 13, 2019
@miolane
Copy link
Contributor Author

miolane commented May 13, 2019

Sorry for my previous stupid mistake.

@ura14h
Copy link
Contributor

ura14h commented May 14, 2019

I feel it is better to create a link using getURL() at the time of display like app/ui-flextab/client/tabs/uploadedFilesList.js, not at upload time.

@tassoevan what do you think about?

Copy link
Contributor

@tassoevan tassoevan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@miolane, @ura14h is absolutely right; fixing the bug at the getURL() helper is the way to go.

@miolane
Copy link
Contributor Author

miolane commented May 14, 2019

@ura14h Thanks for your direction. I'm not familiar with the frameworks and the whole system and do it in a wrong way.

@miolane
Copy link
Contributor Author

miolane commented May 16, 2019

@tassoevan I found many wrong urls in this edge case. One of them is the footer of side navigator. I tried to change app/lib/server/startup/settings.js at line 1246 to

return this.add('Layout_Sidenav_Footer', `<a href=${ getURL('home') }>\<img src="assets/logo"/\></a>`, {
			type: 'code',
			code: 'text/html',
			public: true,
			i18nDescription: 'Layout_Sidenav_Footer_description',
		});

but it doesn't work

@ura14h
Copy link
Contributor

ura14h commented May 17, 2019

@miolane Your insight is great.

However, I think it is difficult to solve as this pull-request because the scope is too large. This pull- request should be focused on only solving the upload and download link issues, for to merge easy.

You can report a new issue about other wrong links you found and create a new pull-request for to fix it.

Thanks.

@miolane
Copy link
Contributor Author

miolane commented May 18, 2019

@ura14h I have already solved the upload and download issue by using getURL() but it still tells me
"Changes requested". I don't how to let it be merged. Sorry I'm new in Github.

@ura14h
Copy link
Contributor

ura14h commented May 19, 2019

I think that the reviewer will close "Changes requested" manually. So you need only to mention to tassoevan that "changed to using getURL()" after fixing the point where He and I pointed out.

Then after, reviewers reviews your changes again, and RocketChat development members approve and merge it into develop-branch.

@ura14h
Copy link
Contributor

ura14h commented May 19, 2019

@tassoevan, I feel that 3455ad1 is good but 22b7848 is not necessary.

@tassoevan tassoevan added this to the 1.1.0 milestone May 20, 2019
@rodrigok rodrigok changed the title [FIX]fix uploading file (#14472) [FIX] Download files when running in sub directory was not working May 20, 2019
@sampaiodiego sampaiodiego changed the title [FIX] Download files when running in sub directory was not working [FIX] Downloading files when running in sub directory May 20, 2019
@sampaiodiego sampaiodiego merged commit 5178873 into RocketChat:develop May 20, 2019
@miolane miolane deleted the develop-uploadfile branch May 27, 2019 06:42
wreiske added a commit to wreiske/Rocket.Chat that referenced this pull request May 27, 2019
… into new-reports-ui

* 'new-reports-ui' of https://github.com/wreiske/Rocket.Chat: (97 commits)
  LingoHub Update 🚀 (RocketChat#14643)
  [FIX] Role name spacing on Permissions page (RocketChat#14625)
  [FIX] Avatar images on old Livechat client (RocketChat#14590)
  [FIX] Inject code at the end of <head> tag (RocketChat#14623)
  [FIX] Mailer breaking if user doesn't have an email address (RocketChat#14614)
  Ci improvements (RocketChat#14600)
  [FIX] E2E messages not decrypting in message threads (RocketChat#14580)
  Fix: emoji render performance for alias (RocketChat#14593)
  [FIX] Send replyTo for livechat offline messages (RocketChat#14568)
  Federation i18n message changes (RocketChat#14595)
  [REGRESSION] Fix Slack bridge channel owner on channel creation (RocketChat#14565)
  Fix thumbs up emoji shortname (RocketChat#14581)
  Fix broken logo url in app.json (RocketChat#14572)
  Add digitalocean button to readme (RocketChat#14583)
  Fix: Add emoji shortnames to emoji's list (RocketChat#14576)
  [IMPROVE] Message rendering time (RocketChat#14252)
  [IMPROVE] Change user presence events to Meteor Streams (RocketChat#14488)
  Removed unnecessary DDP unblocks (RocketChat#13641)
  [FIX] Downloading files when running in sub directory (RocketChat#14485)
  [FIX] Broken layout when sidebar is open on IE/Edge (RocketChat#14567)
  ...
@sampaiodiego sampaiodiego mentioned this pull request May 28, 2019
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.

Can't download files when running RocketChat in subdir
5 participants