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

CRM-21225: Added a new field, Profile Title so that the Profile Name does not serve a dual purpose of labelling the Profile and displaying the Title for the Profile fields publicly. #11370

Closed
wants to merge 1 commit into from

Conversation

agilewarealok
Copy link
Contributor

@agilewarealok agilewarealok commented Dec 5, 2017

Overview

Add a new field, Profile Title so that the Profile Name does not serve a dual purpose of labelling the Profile and displaying the Title for the Profile fields publicly.

The use case is when you want a Profile to have a name which is used internally and describes how the profile is being used but want to use a different title when the profile is embedded on a contribution, event etc.

eg. "Top secret donors" = Profile Name, versus "Enter your donation details" = Profile Title
Agileware Ref CIVICRM-506

Before

Profiles had only single field to show internally and publicly.

After

Now profiles have name as well as title, we are using display title for displaying the Title for the Profile fields publicly.

Agileware Ref: CIVICRM-506


@seamuslee001
Copy link
Contributor

Hi @jusfreeman There are a few issues here 1. the xml file changes aren't included nor are the changes to sql/civicrm_generate.sql also doing upgrade testing i was finding that this wasn't properly adding the fields in multilingual situation. It also showed a bug in the code as well. I have addrsssed all the issues here seamuslee001@0a4bb57577

@jusfreeman
Copy link
Contributor

Thanks @seamuslee001 I'll ask @agilewarealok to review.

@JoeMurray
Copy link
Contributor

I'm in favour of this schema change. I haven't reviewed patch.

@jusfreeman
Copy link
Contributor

@seamuslee001 any idea why the build is failing for this PR? The link to the build report is broke.

@seamuslee001
Copy link
Contributor

@jusfreeman i think it may have been conflict or something. I have started work on getting the schema changes through as needed here #11503

@jusfreeman
Copy link
Contributor

@seamuslee001 great thanks, will you be able to manage this PR to final merge? Or do we need to do more stuff with it?

@seamuslee001
Copy link
Contributor

@jusfreeman i'll try and manage this one through, there was some chatter on the isssue on MM this morning to resolve the naming of the new column which should be done now

@totten
Copy link
Member

totten commented Jan 11, 2018

Discussed this a bit on Mattermost with Eileen and Seamus. On general principle, I agree there's utility in splitting apart the profile title used for backend/admin forms (like "Edit Event" or "Edit Contribution Page") and the profile title used for frontend/standalone profiles. That could potentially be accomplished by splitting off a new field for the backend titles or frontend titles, and (if I'm interpreting correctly) I think this makes the right call in splitting off the frontend titles. (There are fewer pages/use-cases that would need to be updated.)

Those comments aren't a full review, though -- a full review would, e.g., need to r-run it and examine the fallback/transition paths for existing data.

@jusfreeman
Copy link
Contributor

@seamuslee001 thanks mate, appreciated.

@totten we weren't really thinking of changing all backend/admin forms, just dealing with the profile use case. Wary of expanding the scope too much and putting noses out of joint. The original issue description, use case is what we're trying to solve here.

I'll leave this one in your respective safe hands :)

Thanks!

@agilewarealok
Copy link
Contributor Author

Closing this one as similar PR(#11508) is merged into master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants