-
Notifications
You must be signed in to change notification settings - Fork 20
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
Make Create Project Flow descriptions configurable #2198
base: dev
Are you sure you want to change the base?
Make Create Project Flow descriptions configurable #2198
Conversation
[T-CAIREM 1137]
Thanks @matkaczmarek please could you include a full description for the pull request here? I don't think we have access to your issue tracker (or at least if we do, not all of us are using it!). |
Hi @tompollard I updated the description but keep in mind that this is just a proof of concept and still in WIP state. We will discuss the general approach to this feature with @Rutvikrj26 on Tuesday but I created this draft to demonstrate one of the potential approaches. |
[T-CAIREM 1137]
[T-CAIREM 1137]
Two questions for @tompollard, @bemoody:
Also I'm not very fond of naming those new class as Step/StepDetails, maybe you have better idea? |
(I haven't looked at the code at all, just responding to things we talked about in the meeting.)
Not a great idea for a couple of reasons:
A better approach is to move the default text into the database (using a data migration.)
Yeah, these don't really belong together and trying to use an abstract class makes things needlessly complicated in my opinion.
Words that come to mind: "form description", "help text", "boilerplate"? I think that we would probably like to reuse the same model for things across the site that are not necessarily tied to the project-submission-workflow pages (for example, the user settings pages.) |
…for adding and modifying the copy for create project workflow
I've completed the implementation started by @matkaczmarek with most of the feedback incorporated by @bemoody. However, I could not come up with a better name and have kept it the same. [Further work on fixing tests pending] @bemoody I do have the fixtures, but need some help with incorporating the fixtures (project_copy.json) in the physionet/fixtures into the migrations at the start of the deployment. Please let me know how would you like that to be implemented for physionet. |
…onsole test defaults for new url paths
@bemoody I have re-generated the migrations as you suggested and it has resolved the issue. The PR is now open for review. Further, this would require data seeding to put into the database at the start of the deployment right after table migrations. Are there any ways that you suggest we implement for that? |
I keep coming back to thinking about translation, because the problem you're trying to solve here is very nearly the same problem that GNU gettext, and other i18n libraries, are meant to solve. If you haven't ever used gettext, you should read about it and understand how it works and why. One of the distinguishing features of the GNU approach is that the message-ID is the same as the "untranslated" string (i.e., English is treated as the default.) This is a bit controversial; a competing philosophy says that message-ID should be a fixed identifier of some kind, and English should be treated the same as any other language. But either approach recognizes that the messages are separate from the application logic, and they may evolve separately. Suppose, for the sake of argument, that we wanted to move the Ethics Statement into the Content page, while moving the Supporting Documents to a new page after the Files page. Then, suddenly, the former description for the Ethics page might or might not make any sense; not to mention, the order of "steps" wouldn't match what's in the database. As developers implementing this change, we would need to define a new set of defaults, while also trying to preserve existing customizations. That's, in essence, the problem that tools like xgettext/msgmerge are designed to solve. I don't have a completely fleshed out idea here, but hopefully this is some food for thought. |
Summarizing the discussion during the standup, following two key points were mentioned:
@bemoody please add any points I might have missed during the discussion. |
@bemoody @tompollard This is becoming a critical feature as more and more people uploading databases are getting confused. We would like to have this one merged as soon as possible. Is it possible to merge this one and refactor later once the feature is out there? |
T-CAIREM 1137
We should make copies on create project flow easy to manage and change: