Magento Architectural Discussions, 17 Jul 2019

Previous Meeting Next Meeting

⏯

youtube image

►

From YouTube: Magento Architectural Discussion -- July, 17, 2019

Description

* Versioning policy updates
* Domain Whitelist for Configurable 3rd Party Redirects

Meeting notes - https://github.com/magento/architecture/issues/205

A

So an architectural discussion meeting and Danielle can you start from your topic.

B

He cannot hear yourself.

A

B

Try to solve this issue, okay,.

A

Do you want to start I.

B

Will have to go in five minutes? Oh no! Okay,.

A

We can switch to the second topic for now.

A

So I have the second topic: it's about versioning. All these things are missing.

A

There are some new questions.

A

Those are related to inflatable phrases and database changes.

A

Voice changes to review, but let's, let's discuss so for translatable phrases in the backward compatibility guideline. We already found an item that we should not modify, translatable phrase, I, also added, modify or add, and it's relatable price and I updated, called voice changes.

A

So boy I added messages, section practices.

A

Like this I'm Jim change goes to database, the messages, translatable, phrase modified or other it's a major change and phrase remove which it's a price change. Some explanation to this is why it's major change, because, as soon as somebody adds or modifies existing translatable phrase, meaning the key for the translation, it will lead to untranslated phrase in the production score for the mesh for the merchant. So it should be. It should be visible from the word and change that something happens and they need to update translations.

A

So that's why I think.

C

Adding a modification.

A

Might change again in mind or magenta releases really well, we can go and update the phrases. It's more expected in.

A

A

But at least it will be reflected in the module version, so merchants can.

A

Another opinion might be that this will cause comfortability with extensions, which is not.

A

D

Point of view, my chest is too strict.

D

I proposed to me, we not measure per minor because all functionality will work and for most people who were this understood in English. It's okay to see the phrase but without translate.

A

Okay, so for me my needs either backwards-compatible change or spi change in spss change is an extension points. It's not an extension point and it's not backwards compatible from the functionality perspective. That's why I put it as major I read I.

D

Believe our record compatibility purple is not about functionality. It's about what compatibility api compatibility, because any changes change functionality in any case.

B

New phrase doesn't break code, yes, will show English and they might not be.

A

Think I think version should reflect both functional and technical change, not on the technical change. It's not only about having fatal error on the website. It's about any change is.

E

Adding a new one, breaking though like if we're talking about like you know if we were talking about front-end stuff, just as a way to compare like there's the whole concept of progressive enhancement, right, there's a clear distinction between something works. But but maybe it's not 100% the ideal world versus something straight-up breaks, and if something doesn't prevent a shopper from going through any of the scenarios they're supposed to go through, it seems like making something breaking for something that doesn't break any scenario.

B

F

B

F

We have use cases around new phrases added for exceptions, at least from what I can see in watch releases. We don't usually edit for new functionality but for exception messages which I think is.

A

F

F

Introduce new exceptions and they want to create better message. They want to create.

F

Exception, which is supposed to be shown on stuff on.

G

Validation messages are the common cause. Yes,.

A

Yeah I think messages that are shown to the user. Yes, for exceptions, it's question about how we can do it, especially with this localized exception.

A

F

Separate these two use cases, one use case for new functionality and positive flow, which may be major because users will see untranslated phrase in normal flow and second is exceptional case, which may be.

A

F

Let's assume, you add new phrase to item successfully added to cart, so you change this mushy stuff message somehow means that every user on the website will see under static phrase in positive scenario. In case you modify phrase related to some exception. It will only affect users wish.

A

So what do you propose major versions for positive scenario and minor version? They're.

F

A

F

Like minor for it for some translations.

B

In our case, it prevented us to build a better API on the graphical coverage, because we couldn't introduce new phrases and those phrases weren't them for the consumption of the front-end. Well, API had very limited entity. Does not.

B

We couldn't provide a better messages, because.

B

A

So they understand nobody thinks that it should be a fun change. Right.

A

Majority of us thinks that it should be a minor change.

A

G

What is the workaround then for say a bug fix where you have to introduce new validation or something and there's no existing validation messages that would be suitable.

A

Discuss with product owner and with everybody and active managers or architects, and just raise major minor.

G

A

But really haven't watched.

A

Versioning is for community for extension and customization developers to know that something changed right for us. We can have a discussion and decide what is what will be a better approach introduced, a major minor change or preserve all not functionality.

A

D

A

Understanding is that the most of us think that it's better with a minor version change for the prices. Let me maybe collect more feedback from the community on this. If, if no objections will be found.

A

G

Can you also explain why the removal of the translation is only a patch.

A

Cuz I, don't think somebody should be relying so, for example, checkout should not rely on catalogs text.

A

So if you remove it, it means you probably remove the functionality. So removal of the phrase itself is not a breaking change.

A

By some reason, like by a mistake, if you remove it, it will not impress anybody so I. Consider the phrase itself is being a private call. Nobody should rely on it.

A

H

A

Had temporary tables change, it's not clear whether it's about like real my signal, temporary tables or about something tables that does understand we have which are not like.

G

Really temporary tables.

I

B

A

From parallel tables, so I added some clarification here that it doesn't know if it's for any table that is considered temporary, but another big addition is engine change, I'm.

C

A

Fond of you right here, I could major, but I need some feedback. Probably what are their opinions about engine.

B

A

D

Know in magenta we have functionality that the motto is the change game of engine for each number of tempo dynamically so actually enable some modules you'll.

D

Change enzyme and major break.

A

Okay, so yeah the question about it: it's a breaking change, I,.

D

Think is breaking change. Ways by someone depends on unjoin change, I believe the problem only give some compile incompatibility between joins, for example, Simon trying to not support transaction. In this case it will be break, but it's.

A

A

A

Okay, I guess: that's all I have something back I'll update. So the question is: if from that maybe have been nail.

J

Yeah, can you hear me?

J

Okay, so yeah we have a proposal to add a new configuration for a list of white listed domains, because so we were implementing payment methods for PayPal in graph QL and we're trying to implement it in a way where the front-end and back-end application can be decoupled. And in that scenario it was brought up that there could be the potential that they're on different domains. So the redirect redirects the front-end and currently all the logic for like where to redirect it is in the backend.

J

So basically we need to accept from the front-end application a URL to redirect after the payment is successful or erred or anything and so secure that we're gonna introduce this new list of domains that we can validate the input URLs against. Oh.

J

I guess I should share my screen and bring it up and.

J

You see, okay,.

J

See my screen: yes, the okay, it's not shown in my please so there's been discussion about where exactly the configuration should live. Originally we suggested an EMV dot, php' I'm thinking it some more secure. It's only editable Vic. If you can edit the file or run out of CLI command, Alex suggested it just being in the regular store configuration I.

J

Think we could, that could still be up for discussion. It doesn't doesn't impact the overall idea too much, and then there was another more discussion about whether or not it's even feasible to have a front-end application in the back-end application on different domains.

J

Think maybe someone maybe Andrew kin or Andrew or uh Eugene, talked to that more yeah.

E

My comment was just about the fact that it seems like we're doing this work now for like the the winner that, if that someone runs the the graph QL API on a separate host name or host port protocol combo and the problem with that is with Magento today in the state of web browsers in 2019, you can't do that.

E

The only way that you'd be able to run the graph QL API on a separate host name would be switching to token based off instead of cookie based off, and it sounds from talking with yevon that that might be a far future type thing. So I'm, not necessarily trying to say we shouldn't do this. I just I want to make sure that we, you know proposals. One thing versus like implementation is another thing so like this work makes sense. Coupled with you know, if at some point in the future, we do do that.

E

I just wanted to bring up that. This might be something that we end up. It could end up making security looser if we don't actually need feature in the future. So I just wanted to make sure to raise that point, because if we're like, you know for three or four years away from being able to do token off with the API, then we're far ways away from this being able to be usable by people.

G

I

Even it's not about it's not only about different domains. It's on. It's also about different URLs, because, for example, like the original example was related to the payment integration and the Ken doesn't know about the euro, which Europe euros should be used for a direction only front end knows about it so and front end. Somehow should provide this URLs in secure way. Actually, the instances could be on the same domain, but the URLs can be different, and somehow we need to solve this problem.

J

So for now, does it make sense to just limit the user supplied URL to their route and use? The car like Andrew, suggests, use the scheme host and port from.

H

Not a good visual.

B

The problem is that if we already released the page.

B

If we don't do something about it, we have to revert all of them by two three I think that's the case on this problem.

J

Or, or limit the URL and put to just the route and use the hostname that's configured yes,.

B

We have to do something about it, so.

K

E

My personal opinion is that we should go with whatever permits the least the least amount of variable URLs in there, and so we have a use case that we know can be actually like utilized for it right. So personally, I'd be 4i, I can't say, I necessarily understand what or if there'd be a difference between base URL and the host port protocol that the API is called under, but yeah.

E

My own personal opinion would be that until we have a use case where we know that the front-ends could functionally be under a different host name, that we'd want to just pin it to the single thing we know would be used, but that's you know, obviously an opinion enough for discussion.

A

Now, let's see what happens to implement that, and sometimes we need to still call with some white listing or we need to support other domains to bid I think we can add it in pretty backward-compatible way right. So we will add it as a new feature and still if no the mine is provided or no URL and whitelist is provided. We can fall back to to the current demand of graticule right.

J

So you're saying limit the input to not include like the domain to not allow it to redirect to other domains and if, in the future, we have that as a viable set up, we can at that type of functionality. Yes,.

A

Yes, I'm just thinking.

J

How problematic.

A

It will be to add this functionality in future and to me it seems it shouldn't be a big problem. It.

J

Seems like a knowing the additives yeah.

A

So I agree with Andrey. If you don't have real use case right now, maybe let's just limit it. It will be more limited, so it should be more secure, probably them adding more options and probably easier to implement.

J

I

Actually, right now, for example, pwe will specify the domain if, if we have only one place for domains, admin configuration.

A

So yes, we'll use the same to minus it, as the back end will use its own domain. So.

I

Graph kill, you will compare the domains and.

A

Yet only question is that you mentioned we, it doesn't know which, like your else, posts are allowed. I, don't know if it's a big problem. If.

I

The same demony now it shouldn't be a problem because, like it will be anywhere directed to something with magenta notes or or doesn't know there.

E

Is there is one problem, I just realized that we're gonna have with this, but I don't want to take up too much of the call time with it. So I'll I'll come find Daniel afterwards and then we'll talk it out and get the ticket updated. We're probably not going to talk to the PWA folks of it.

A

No do we have anything else to discuss for today, if not I think we are done and hopefully we'll have, a woman will have normal link to blue jeans next time. Thanks so see you.