search

dasch-swiss / beol

Bernoulli-Euler OnLine
https://beol.dasch.swiss
GNU Affero General Public License v3.0
0 stars 1 forks source link

Component for the newton project #108

Closed SepidehAlassi closed 5 years ago

SepidehAlassi commented 5 years ago

solves #106

SepidehAlassi commented 5 years ago

@tobiasschweizer tests are fixed

tobiasschweizer commented 5 years ago

tests are fixed

Great! I will get the review done by the end of this week. I am excited to see content from a third party repo integrated into BEOL :-)

SepidehAlassi commented 5 years ago

@tobiasschweizer 😄On Friday Lukas wants to show that Newton connection works. I guess we might want to at least get the test server on this branch by tomorrow.

subotic commented 5 years ago

Are you going to merge into develop today? Deploying it on the test server shouldn't take long.

tobiasschweizer commented 5 years ago

@SepidehAlassi Could you please check https://github.com/dhlab-basel/beol/pull/108#discussion_r252578921? I think this could cause a problem on the test server.

If this is checked/fixed, please go on and deploy this branch on the test server.

tobiasschweizer commented 5 years ago

@SepidehAlassi Do you think we could replace cors-anywhere herokuapp with an implementation in Knora some day?

I think this solution is perfect for the moment, but on the long run this dependency could be problematic.

SepidehAlassi commented 5 years ago

I think this solution is perfect for the moment, but on the long run this dependency could be problematic. @tobiasschweizer yeah, I am aware that this can be problematic in long run, but for now it is the easy solution.

SepidehAlassi commented 5 years ago

I have a problem displaying the facsimiles for NATP00291: http://cudl.lib.cam.ac.uk/view/MS-ADD-09597/255 does not work (page not found)

Currently newton xmls are not updated with the cul changes. Michael is working on it. In the next import, the URIs of the facsimiles will be updated and these errors will be solved by then.

SepidehAlassi commented 5 years ago

@subotic we are going to merge this branch later. But the test server should be using this branch. Can you please update the test server?

subotic commented 5 years ago

@SepidehAlassi @tobiasschweizer done

SepidehAlassi commented 5 years ago

@subotic Danke 👍

SepidehAlassi commented 5 years ago

@subotic as a very polite, humble request: " can you please update the beol test server with this branch, please?" 😉 I have added the mathJax rendering to Newton letters, it might impress some people 😄

subotic commented 5 years ago

@SepidehAlassi done :-)

SepidehAlassi commented 5 years ago

@subotic There is a problem though with the path of the images embedded in html body. After fetching the body of the newton lette, I replace the environment.app URI with the newtonproject URI, locally the replacement is correct and I get the following result,

screenshot 2019-02-04 17 03 46

But the replacement of the URI fails in test server

screenshot 2019-02-04 17 05 32

is the environment.app not the one test server is using?

kilchenmann commented 5 years ago

environment.app is for the configuration of the Angular app url, wich can be different in each environment. When you change it into the url of the newton-server, we will have 404 errors in components or services where we need the Angular app url. The problem in the html of the Newton page is as follow: in case of an image source, it‘s not the whole url, it‘s only the relative path there... but I‘m not sure and I will have a look at it. Can you send me two urls please: one from a desired source on the newton server, and one from the local beol app, where this newton source is displayed. I've got it

kilchenmann commented 5 years ago

@SepidehAlassi I see now how you did it (yesterday it was too late):

if (image.src) {
    image.src = image.src.replace(environment.app, 'http://www.newtonproject.ox.ac.uk');
    console.log(environment.app);
}

Nice idea but I'm not sure if that's the best practice. But we will figure out, how we can solve it 😉

kilchenmann commented 5 years ago

@subotic are you using the config.dev.json for the deployment? We should try to fix the app configuration issue here in beol... What do you think?

subotic commented 5 years ago

no, I have two custom environments which I copy into the source code. Afterwards I build the app. Maybe we could change BEOL to use config.xxx.json? They don’t use the firestore stuff anymore. Then @SepidehAlassi could easily change the config at runtime.

SepidehAlassi commented 5 years ago

@kilchenmann and @subotic Thank you very much guys for thinking about this problem, I'll appreciate any solution (dirty or neat 😉 )

kilchenmann commented 5 years ago

@subotic can we do that together? next Monday and in a separate branch?

subotic commented 5 years ago

yes, sure.

tobiasschweizer commented 5 years ago

@SepidehAlassi I propose the following procedure:

Is that ok? I will do the review as soon as this PR is ready.

SepidehAlassi commented 5 years ago

@tobiasschweizer The problem which Andre and Ivan are working on should be solved first before we merge this branch to develop.

tobiasschweizer commented 5 years ago

The problem which Andre and Ivan are working on should be solved first before we merge this branch to develop.

I understand. How much time do they need appr.?

SepidehAlassi commented 5 years ago

I understand. How much time do they need appr.?

not sure, they had planned to work on it this week. @subotic & @kilchenmann any update/progress with config setup for beol?

subotic commented 5 years ago

@SepidehAlassi we got it "almost" working yesterday. I hope @kilchenmann can find some time this week to finish it.

SepidehAlassi commented 5 years ago

@SepidehAlassi we got it "almost" working yesterday. I hope @kilchenmann can find some time this week to finish it.

@subotic & @kilchenmann great, thank you guys. 😄

kilchenmann commented 5 years ago

@subotic & @kilchenmann any update/progress with config setup for beol?

It's done and works in knora-ui modules. @subotic is testing it in his own app and then we can use the same setup in the beol app.

SepidehAlassi commented 5 years ago

It's done and works in knora-ui modules. @subotic is testing it in his own app and then we can use the same setup in the beol app.

That is great news. Thank you very much.

tobiasschweizer commented 5 years ago

There is still a problem for the productive build: https://github.com/dhlab-basel/Knora-ui/issues/178

SepidehAlassi commented 5 years ago

@tobiasschweizer this can be reviewed now, you can use the old data for it.

tobiasschweizer commented 5 years ago

@SepidehAlassi ok, I will do it tomorrow.

SepidehAlassi commented 5 years ago

ok, I will do it tomorrow.

@tobiasschweizer thanks

SepidehAlassi commented 5 years ago

@tobiasschweizer sorry, I just remembered you can not use the old data to check this. I have uploaded new data in switchdrive, please use the data there to check this pull request.

tobiasschweizer commented 5 years ago

@SepidehAlassi So this is a step towards an interoperable edition platform. Congratulations! :-)

SepidehAlassi commented 5 years ago

@tobiasschweizer thanks 😄