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

Meteor Email Admin Server Error Solution #89

Open
wants to merge 19 commits into
base: development
from

Conversation

@Max2020q
Copy link

commented Jun 12, 2019

Fixes issue with loading 404 page version of website correctly

Example: (ogv.com vs. ogv.com/test)

  • This will allow the site to load even if the file description ending is named incorrectly, allowing for more usability.
404 Error Page Fix
Fixes issue with loading 404 page version of website correctly 

Example: (ogv.com  vs. ogv.com/test)
- This will allow the site to load even if the file description ending is named incorrectly, allowing for more usability.
@Max2020q Max2020q referenced this pull request Jun 12, 2019
@inderpreetsingh
Copy link

left a comment

Just need some questions answered. Thank you for the PR.

imports/ui/components/error.css Outdated Show resolved Hide resolved
imports/ui/components/error.html Outdated Show resolved Hide resolved
@Gauravjeetsingh

This comment has been minimized.

Copy link
Collaborator

commented Jun 19, 2019

@Max2020q I see, you copied the landing page in the error template. That's not a good approach to fix it. What you can do here is show an error page with a link to a landing page.

And, you don't have to create a new design for the page. We already have a 404 page (OGV-meteor/imports/ui/pages/404.html),

The issue you need to fix is:

  • Use the template notFound in file 404.html on 404 error
  • Figure out why the link to the home page is not clickable in that template.
@Max2020q

This comment has been minimized.

Copy link
Author

commented Jun 20, 2019

Hello, thank you for the advice, I have reviewed the comment!

The current 404.html to me is not as good as the landing page.html.

https://github.com/BRL-CAD/OGV-meteor/blob/development/imports/ui/pages/404.html can be replaced by the https://github.com/BRL-CAD/OGV-meteor/blob/development/imports/ui/pages/landingPage.html instead

I chose the error.html approach because routing is not working on 404.html in a lot of aspects.

Most errors seem to route to error.html right now including nature 404 errors..

@Max2020q

This comment has been minimized.

Copy link
Author

commented Jun 21, 2019

I am going to revert these changes and fix the 404.html template instead and the routing.

Max2020q added 3 commits Jun 24, 2019
Readding 404 page
Routing for the 404 page doesnt seem to work from pages section..

The way its set up in app.js, the 404 errors automatically go to error.js.

Even with the onClick function, it seems to not be moving to the new page, this may be due to z-index on the frame for the 404 page.
@Max2020q

This comment has been minimized.

Copy link
Author

commented Jun 24, 2019

Added some more changes. Right now in OGV-meteor error.js is set up in iron-router as the default 404 page.

Because of this, my solution involved adding the 404.html to the error.html, centering the style, and trying to fix the link to home page.

Max2020q added 4 commits Jun 27, 2019
Fix Landing Page Layout
Got rid of unnecessary landing page error model loading.
<div class="col-6">
<iframe width="350" height="250" src="/models/{{landingPageModel}}/shared=true" frameborder="0"></iframe>
</div>
{{/if}}

This comment has been minimized.

Copy link
@Gauravjeetsingh

Gauravjeetsingh Jun 28, 2019

Collaborator

We need this model on the landing page. Do not remove it.

Currently, you used the error template for 404-page error. But this template is also being used to show when model is not displayed. So we will get this 404 error, when there is a problem in the model rendering. In the screenshot below, you can see how we got this on home page as well.
Screenshot from 2019-06-27 17-07-09

This comment has been minimized.

Copy link
@Max2020q

Max2020q Jun 28, 2019

Author

I would like to have a more in-depth discussion about this, because that model has not loaded and will not load on the landing page properly this whole time so I removed it.

The 404 page works correctly in other instances.. I believe the real problem is separating the model loading error from the 404 error, which is the same at the moment, that may require a separate PR to separate that, but for now I believe this works for the 404.

I just have to fix the model loading in a separate PR. Panda please let me move on to fixing that because the 404 page works as of now, I just have to fix the model loading error separately, their are a lot of other errors regarding model loading such as scale of models and error notification as well.

This comment has been minimized.

Copy link
@Max2020q

Max2020q Jun 28, 2019

Author
<iframe width="350" height="250" src="/models/{{landingPageModel}}/shared=true" frameborder="0"></iframe>

Is not properly calling that model so I temporarily got rid of it until I get a proper solution for loading that model. I think fixing that should be in a seperate PR and issue solving for the landing page model loading issues.

Max2020q added 4 commits Jul 11, 2019
Added Required Setting isAdminEmailServerOn
Goal: Do not allow the creation of user account until the admin has set email server.
Added Required Setting isAdminEmailServerOn Signup
Goal: Do not allow the creation of user account until the admin has set email server.

@Max2020q Max2020q changed the title 404 Error Page Fix Meteor Email Admin Server Error Solution Aug 1, 2019

Max2020q added 3 commits Aug 1, 2019
const requiredUsername = Meteor.settings.public.smtp.username;
const requiredServer = Meteor.settings.public.smtp.server;
const requiredPassword = Meteor.settings.public.smtp.password;
if (requiredUsername.includes(mailgun.org)&&requiredServer.includes(mailgun.org)&&requiredPassword.length>0) {

This comment has been minimized.

Copy link
@inderpreetsingh

inderpreetsingh Aug 2, 2019

I think you are missing a few quotes in this if condition.

This comment has been minimized.

Copy link
@Max2020q

Max2020q Aug 3, 2019

Author

Thank you for this review, I have added the quotes and changed the comment above for the function.

Max2020q added 3 commits Aug 3, 2019
Fix Login and Sign Up not working from last commit
Solves solution for Admin Email Server Issue.
Matching previous commit on other PR for same issue
I changed the name and subject of this commit from the 404 issue to the email server issue. Instead of deleting the account.js file which was apart of the previous solution to this PR before  I changed the name, I am matching it to the solution for the new account.js in the patch-1 branch which solves the issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.