Skip to content

Create App: Broken Url Report #1069

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

Merged
merged 18 commits into from
Apr 1, 2021
Merged

Create App: Broken Url Report #1069

merged 18 commits into from
Apr 1, 2021

Conversation

AleziaKurdis
Copy link
Contributor

This PR add a new functionality to the Create App.

Generate Broken Url Report:

Under the "Tools" menu, we have now a "Generate Broken Url Report"

This functionality scans all the URL properties of the selected entities, it tests each of them and generates a report listing all the invalid URL found.

image

image

The following properties are covered by this functionality:
"script"
"serverScripts"
"imageURL"
"materialURL"
"modelURL"
"compoundShapeURL"
"animation.url"
"textures"
"xTextureURL"
"yTextureURL"
"zTextureURL"
"font"
"sourceUrl"
"scriptURL"
"filterURL"
"skybox.url"
"ambientLight.ambientURL"

Note:
This report is specific to HTTP URL.
This report ignores URL from the Asset Server (atp://), local drive paths or any string not starting by 'http'.

Changes to Menu bar:

The Hover color of the menu items of the menu bar have been also modified from cyan to gray since it was visually annoying when it was used over a selection of entities in the list. (disturbing cyan over cyan)

This change the hover color for the menu item of the menu bar
previously it was blue-cyan as the selected entities which was visually annoying when we used the menu 
over a selection of entities in the list.
This hover color is now a grey tonality.
@AleziaKurdis
Copy link
Contributor Author

This addresses the issue #1051

@digisomni digisomni added this to the 2021.1.1 Eos Release milestone Feb 27, 2021
The menu separator wasn't full width.
Add a Link to select the entity.
So the user can edit each entity listed in the report
by clicking on it. This select the entity.
@AleziaKurdis
Copy link
Contributor Author

Added links in the report to select each entity directly in order to let the user to edit.

image

@Aitolda Aitolda added the needs testing (QA) The PR is ready for testing label Mar 1, 2021
@Aitolda
Copy link
Collaborator

Aitolda commented Mar 1, 2021

So I ran a report on an old domain and sure enough, 10 items came up. So far so good, though I have a couple thoughts.

  1. I realize the entity list isn't setup this way nor would it be performant, but what would have to change to be able to create the report on all entities in the domain, regardless, rather than just what you have pointed your camera in the direction of? I should clarify that I don't think that behavior should be the norm, but specifically when running reports.
  2. I think a quick explanation of each "status" might be handy. I know most of us are used to seeing 404 on websites, but I don't know what a zero status indicates.
  3. Could we possibly save a copy of this report somehow?

@Aitolda Aitolda removed the needs testing (QA) The PR is ready for testing label Mar 1, 2021
@AleziaKurdis
Copy link
Contributor Author

Now I display the Error text in addition to the Error no. (Point 2)

image

For the point 1 and 3:
The API is not geared to return all the entities. It's a current limitation and it would be complex to change. (If we want this, we should consider a tool to work on the model.json file of the domain directly (which is another project)

Save is not possible. (for now) It never been for security reasons. If we come to support a safe way to do it, this could be added.
A copy to clipboard could be another option but I'm not sure it would be convivial.

@Aitolda
Copy link
Collaborator

Aitolda commented Mar 3, 2021

Works great!

@digisomni digisomni self-requested a review March 11, 2021 23:10
@digisomni digisomni removed their request for review March 21, 2021 17:13
Broken Url Report - adjustments
Broken Url Report - adjustments
Broken Url Report - adjustments
Broken Url Report - adjustments
Broken Url Report - adjustments
Broken Url Report - adjustments
Broken Url Report - adjustments
@ctrlaltdavid ctrlaltdavid added CR Approved At least one code reviewer has approved the PR. needs testing (QA) The PR is ready for testing and removed needs CR (code review) labels Mar 22, 2021
Copy link
Member

@digisomni digisomni left a comment

Choose a reason for hiding this comment

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

Seems to work alright to me. :)

@Aitolda
Copy link
Collaborator

Aitolda commented Mar 30, 2021

Seems to be working as intended.

@digisomni digisomni added QA Approved The PR has been tested successfully. and removed needs testing (QA) The PR is ready for testing labels Apr 1, 2021
@digisomni digisomni merged commit 519db87 into vircadia:master Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CR Approved At least one code reviewer has approved the PR. enhancement New feature or request QA Approved The PR has been tested successfully.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants