Skip to content

Url shortener#5497

Merged
BigFunger merged 8 commits intoelastic:masterfrom
BigFunger:url-shortener
Dec 18, 2015
Merged

Url shortener#5497
BigFunger merged 8 commits intoelastic:masterfrom
BigFunger:url-shortener

Conversation

@BigFunger
Copy link
Copy Markdown
Contributor

@BigFunger BigFunger commented Nov 24, 2015

Fixes #1553

This is my first attempt at a working prototype for the url shortener, and I assume that it is going to require changes. I'm looking for input on the implementation.

@BigFunger BigFunger self-assigned this Nov 24, 2015
@BigFunger BigFunger assigned tsullivan and unassigned BigFunger Nov 24, 2015
tsullivan
tsullivan reviewed Nov 25, 2015
View reviewed changes
Comment thread src/server/http/urlLookup.js Outdated
Copy link
Copy Markdown
Member

@tsullivan tsullivan Nov 25, 2015

Choose a reason for hiding this comment

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

Can these const client = server.plugins.elasticsearch.client; lines be moved to a single line within the top-level function?

@tsullivan tsullivan assigned BigFunger and unassigned tsullivan Nov 25, 2015
@spalger
Copy link
Copy Markdown
Contributor

spalger commented Nov 25, 2015

@BigFunger keep in mind that we've standardized on snake_case filenames with #5383

@BigFunger BigFunger mentioned this pull request Nov 30, 2015
Closed
@BigFunger BigFunger assigned tsullivan and unassigned BigFunger Dec 2, 2015
@tsullivan
Copy link
Copy Markdown
Member

tsullivan commented Dec 2, 2015

If I create a link to a saved visualization, put that on an iframe somewhere, then delete the visualization, it throws up the "Create a new visualization" screen in the iframe.

screen shot 2015-12-02 at 4 44 34 pm

Not sure if there's a good solution for this. Maybe you could add detail on the warning message for deleting a visualization if there are linked visualizations out there:

screen shot 2015-12-02 at 4 43 34 pm

@tsullivan
Copy link
Copy Markdown
Member

tsullivan commented Dec 3, 2015

I don't think the problem regarding an iframe hosting a link to a deleted visualization is a huge deal.

LGTM

@tsullivan
Copy link
Copy Markdown
Member

tsullivan commented Dec 3, 2015

I just noticed that when we embed a Search in an iframe, there is a lot of tooling available because the Discover page hasn't really been updated for awareness that it's embedded:

image

@BigFunger BigFunger assigned jbudz and unassigned BigFunger Dec 17, 2015
jbudz
jbudz reviewed Dec 17, 2015
View reviewed changes
Comment thread src/server/http/short_url_lookup.js Outdated
Copy link
Copy Markdown
Contributor

@jbudz jbudz Dec 17, 2015

Choose a reason for hiding this comment

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

should this use server.log?

@jbudz
Copy link
Copy Markdown
Contributor

jbudz commented Dec 17, 2015

Functionally LGTM, made some small comments - passing back.

@jbudz jbudz assigned BigFunger and unassigned jbudz Dec 17, 2015
@BigFunger BigFunger assigned jbudz and unassigned BigFunger Dec 17, 2015
@jbudz
Copy link
Copy Markdown
Contributor

jbudz commented Dec 17, 2015

Passing to @panda01 for seconds. Last commit changes LGTM

@jbudz jbudz assigned panda01 and unassigned jbudz Dec 17, 2015
@BigFunger BigFunger mentioned this pull request Dec 17, 2015
Closed
@panda01
Copy link
Copy Markdown
Contributor

panda01 commented Dec 18, 2015

LGTM!

BigFunger added a commit that referenced this pull request Dec 18, 2015
@BigFunger
Merge pull request #5497 from BigFunger/url-shortener
aad699f 
Url shortener
@BigFunger BigFunger merged commit aad699f into elastic:master Dec 18, 2015
@elasticsearch-bot
Copy link
Copy Markdown

elasticsearch-bot commented Dec 18, 2015

Jim Unger merged this into the following branches!

Branch Commits
4.x 4831453, efa6210, a4b831c, 8534e67, d77aafd, 683c582, 1b4ada8, 7a8997d

BigFunger added a commit that referenced this pull request Dec 18, 2015
@BigFunger
initial prototype
4831453 
Fixes #5497
BigFunger added a commit that referenced this pull request Dec 18, 2015
@BigFunger
modified to use hash
efa6210 
Fixes #5497
BigFunger added a commit that referenced this pull request Dec 18, 2015
@BigFunger
Addressed how modules were defined
a4b831c 
Fixes #5497
BigFunger added a commit that referenced this pull request Dec 18, 2015
@BigFunger
Can now share urls without app state in url
8534e67 
Fixes #5497
BigFunger added a commit that referenced this pull request Dec 18, 2015
@BigFunger
hides the embed section on the discover tab
d77aafd 
Fixes #5497
BigFunger added a commit that referenced this pull request Dec 18, 2015
@BigFunger
reorganized code, added clipboard.js support
683c582 
Fixes #5497
BigFunger added a commit that referenced this pull request Dec 18, 2015
@BigFunger
removed link-to-saved-object functionality
1b4ada8 
Fixes #5497
BigFunger added a commit that referenced this pull request Dec 18, 2015
@BigFunger
Added clipboard cleanup, fixed server logging
7a8997d 
Fixes #5497
@cjcenizal cjcenizal mentioned this pull request Aug 1, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

release_note:enhancement review v4.4.0

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@BigFunger @spalger @tsullivan @lukasolson @alt74 @rashidkpc @jbudz @panda01 @elasticsearch-bot